Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

parser,mt: remove CREATE TENANT ... LIKE #129758

Merged
merged 1 commit into from
Sep 4, 2024
Merged

parser,mt: remove CREATE TENANT ... LIKE #129758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 3 additions & 22 deletions pkg/ccl/crosscluster/physical/stream_ingestion_planning.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ func streamIngestionJobDescription(
ReplicationSourceTenantName: streamIngestion.ReplicationSourceTenantName,
ReplicationSourceAddress: tree.NewDString(redactedSourceAddr),
Options: streamIngestion.Options,
Like: streamIngestion.Like,
}
ann := p.ExtendedEvalContext().Annotations
return tree.AsStringWithFQNames(redactedCreateStmt, ann), nil
Expand All @@ -74,11 +73,6 @@ func ingestionTypeCheck(
ingestionStmt.ReplicationSourceAddress,
ingestionStmt.Options.Retention},
}
if ingestionStmt.Like.OtherTenant != nil {
toTypeCheck = append(toTypeCheck,
exprutil.TenantSpec{TenantSpec: ingestionStmt.Like.OtherTenant},
)
}

if err := exprutil.TypeCheck(ctx, "INGESTION", p.SemaCtx(), toTypeCheck...); err != nil {
return false, nil, err
Expand Down Expand Up @@ -117,15 +111,6 @@ func ingestionPlanHook(
return nil, nil, nil, false, err
}

var likeTenantID uint64
var likeTenantName string
if ingestionStmt.Like.OtherTenant != nil {
_, likeTenantID, likeTenantName, err = exprEval.TenantSpec(ctx, ingestionStmt.Like.OtherTenant)
if err != nil {
return nil, nil, nil, false, err
}
}

evalCtx := &p.ExtendedEvalContext().Context
options, err := evalTenantReplicationOptions(ctx, ingestionStmt.Options, exprEval, evalCtx, p.SemaCtx(), createReplicationOp)
if err != nil {
Expand Down Expand Up @@ -175,12 +160,8 @@ func ingestionPlanHook(
// If we don't have a resume timestamp, make a new tenant
jobID := p.ExecCfg().JobRegistry.MakeJobID()
var destinationTenantID roachpb.TenantID
// Determine which template will be used as config template to
// create the new tenant below.
tenantInfo, err := sql.GetTenantTemplate(ctx, p.ExecCfg().Settings, p.InternalSQLTxn(), nil, likeTenantID, likeTenantName)
if err != nil {
return err
}

var tenantInfo mtinfopb.TenantInfoWithUsage

// Create a new tenant for the replication stream.
tenantInfo.PhysicalReplicationConsumerJobID = jobID
Expand All @@ -197,7 +178,7 @@ func ingestionPlanHook(
ctx, p.ExecCfg().Codec, p.ExecCfg().Settings,
p.InternalSQLTxn(),
p.ExecCfg().SpanConfigKVAccessor.WithISQLTxn(ctx, p.InternalSQLTxn()),
tenantInfo, initialTenantZoneConfig,
&tenantInfo, initialTenantZoneConfig,
ingestionStmt.IfNotExists,
p.ExecCfg().TenantTestingKnobs,
)
Expand Down
1 change: 1 addition & 0 deletions pkg/settings/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ var retiredSettings = map[InternalKey]struct{}{

// removed as of 24.3
"bulkio.backup.split_keys_on_timestamps": {},
"sql.create_tenant.default_template": {},
}

// sqlDefaultSettings is the list of "grandfathered" existing sql.defaults
Expand Down
35 changes: 5 additions & 30 deletions pkg/sql/create_tenant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,33 +13,22 @@ package sql
import (
"context"

"github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/errors"
)

type createTenantNode struct {
ifNotExists bool
tenantSpec tenantSpec
likeTenantSpec tenantSpec
ifNotExists bool
tenantSpec tenantSpec
}

func (p *planner) CreateTenantNode(ctx context.Context, n *tree.CreateTenant) (planNode, error) {
tspec, err := p.planTenantSpec(ctx, n.TenantSpec, "CREATE VIRTUAL CLUSTER")
if err != nil {
return nil, err
}
var likeTenantSpec tenantSpec
if n.Like.OtherTenant != nil {
likeTenantSpec, err = p.planTenantSpec(ctx, n.Like.OtherTenant, "CREATE VIRTUAL CLUSTER LIKE")
if err != nil {
return nil, err
}
}
return &createTenantNode{
ifNotExists: n.IfNotExists,
tenantSpec: tspec,
likeTenantSpec: likeTenantSpec,
ifNotExists: n.IfNotExists,
tenantSpec: tspec,
}, nil
}

Expand All @@ -49,20 +38,6 @@ func (n *createTenantNode) startExec(params runParams) error {
return err
}

var tmplInfo *mtinfopb.TenantInfo
if n.likeTenantSpec != nil {
tmplInfo, err = n.likeTenantSpec.getTenantInfo(params.ctx, params.p)
if err != nil {
return errors.Wrap(err, "retrieving record for LIKE configuration template")
}
}
configTemplate, err := GetTenantTemplate(params.ctx,
params.p.ExecCfg().Settings, params.p.InternalSQLTxn(),
tmplInfo, 0, "")
if err != nil {
return err
}

var ctcfg createTenantConfig
if tenantName != "" {
ctcfg.Name = (*string)(&tenantName)
Expand All @@ -72,7 +47,7 @@ func (n *createTenantNode) startExec(params runParams) error {
ctcfg.ID = &tenantID
}
ctcfg.IfNotExists = n.ifNotExists
_, err = params.p.createTenantInternal(params.ctx, ctcfg, configTemplate)
_, err = params.p.createTenantInternal(params.ctx, ctcfg)
return err
}

Expand Down
131 changes: 0 additions & 131 deletions pkg/sql/logictest/testdata/logic_test/tenant
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,137 +378,6 @@ DROP TENANT two
statement ok
DROP TENANT 'tenant-one'

subtest tenant_templates

query T
SHOW CLUSTER SETTING sql.create_virtual_cluster.default_template
----
·

# Check we can't use the system tenant as template.
statement error using the system tenant as config template
CREATE TENANT othertenant LIKE system

# Create some "interesting" tenant template.
statement ok
CREATE TENANT tmpl;

let $tmplid
SELECT id FROM system.tenants WHERE name = 'tmpl'

statement ok
ALTER TENANT tmpl GRANT CAPABILITY can_view_node_info; -- will be copied
ALTER TENANT tmpl SET CLUSTER SETTING trace.debug_http_endpoint.enabled = true; -- will be copied
-- Simulate resource limits. Will be copied.
-- Note: we cannot use the update_tenant_resource_limits() builtin
-- directly here because it can only be used from a CCL binary.
INSERT INTO system.tenant_usage(
tenant_id, instance_id, next_instance_id, last_update,
ru_burst_limit, ru_refill_rate, ru_current, current_share_sum, total_consumption)
VALUES ($tmplid, 0, 0, now(),
11, 22, 33, 44, ''::BYTES);


statement ok
ALTER TENANT tmpl START SERVICE SHARED; -- will not be copied.

# Use it to create a new tenant.
statement ok
CREATE TENANT othertenant LIKE tmpl

let $otherid
SELECT id FROM system.tenants WHERE name = 'othertenant'

# Verify the service mode was not copied.
query BTTT
SELECT id = $otherid, name, data_state, service_mode FROM [SHOW TENANT othertenant]
----
true othertenant ready none

# Verify the new tenant has the same caps as the template
# (by showing there's no difference between the two)
query TT
SELECT capability_name, capability_value FROM [SHOW TENANT tmpl WITH CAPABILITIES]
EXCEPT SELECT capability_name, capability_value FROM [SHOW TENANT othertenant WITH CAPABILITIES];
----

# Check that the setting overrides were copied.
query TTTT rowsort
SELECT variable, value, type, origin FROM [SHOW CLUSTER SETTINGS FOR TENANT othertenant]
WHERE origin != 'no-override'
----
trace.debug_http_endpoint.enabled true b per-tenant-override

# Check that the resource usage parameters were copied.
query IIRRRRI
SELECT instance_id, next_instance_id,
ru_burst_limit, ru_refill_rate, ru_current,
current_share_sum, length(total_consumption)
FROM system.tenant_usage WHERE tenant_id = $otherid
----
0 0 11 22 33 0 0

# Clean up.
statement ok
DROP TENANT othertenant

# Now set the default template and try again.
statement ok
SET CLUSTER SETTING sql.create_virtual_cluster.default_template = 'nonexistent';

statement error retrieving default tenant configuration template.*tenant "nonexistent" does not exist
CREATE TENANT othertenant

statement ok
SET CLUSTER SETTING sql.create_virtual_cluster.default_template = 'tmpl';

# Create a new tenant - this should use the template implicitly now.
statement ok
CREATE TENANT othertenant

let $otherid
SELECT id FROM system.tenants WHERE name = 'othertenant'

# Verify the service mode was not copied.
query BTTT
SELECT id = $otherid, name, data_state, service_mode FROM [SHOW TENANT othertenant]
----
true othertenant ready none

query TT
SELECT capability_name, capability_value FROM [SHOW TENANT tmpl WITH CAPABILITIES]
EXCEPT SELECT capability_name, capability_value FROM [SHOW TENANT othertenant WITH CAPABILITIES];
----

# Check the setting overrides were taken over.
query TTTT rowsort
SELECT variable, value, type, origin FROM [SHOW CLUSTER SETTINGS FOR TENANT othertenant]
WHERE origin != 'no-override'
----
trace.debug_http_endpoint.enabled true b per-tenant-override

# Check that the resource usage parameters were copied.
query IIRRRRI
SELECT instance_id, next_instance_id,
ru_burst_limit, ru_refill_rate, ru_current,
current_share_sum, length(total_consumption)
FROM system.tenant_usage WHERE tenant_id = $otherid
----
0 0 11 22 33 0 0

# Clean up.
statement ok
DROP TENANT othertenant

statement ok
ALTER TENANT tmpl STOP SERVICE

statement ok
DROP TENANT tmpl

statement ok
RESET CLUSTER SETTING sql.create_virtual_cluster.default_template

subtest regression_105115

statement ok
Expand Down
43 changes: 11 additions & 32 deletions pkg/sql/parser/sql.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,9 +872,6 @@ func (u *sqlSymUnion) showRangesOpts() *tree.ShowRangesOptions {
func (u *sqlSymUnion) tenantSpec() *tree.TenantSpec {
return u.val.(*tree.TenantSpec)
}
func (u *sqlSymUnion) likeTenantSpec() *tree.LikeTenantSpec {
return u.val.(*tree.LikeTenantSpec)
}
func (u *sqlSymUnion) cteMaterializeClause() tree.CTEMaterializeClause {
return u.val.(tree.CTEMaterializeClause)
}
Expand Down Expand Up @@ -1238,7 +1235,6 @@ func (u *sqlSymUnion) triggerForEach() tree.TriggerForEach {
%type <tree.Statement> create_proc_stmt
%type <tree.Statement> create_trigger_stmt

%type <*tree.LikeTenantSpec> opt_like_virtual_cluster
%type <tree.LogicalReplicationResources> logical_replication_resources, logical_replication_resources_list
%type <*tree.LogicalReplicationOptions> opt_logical_replication_options logical_replication_options logical_replication_options_list

Expand Down Expand Up @@ -4744,49 +4740,45 @@ logical_replication_options:
// %Help: CREATE VIRTUAL CLUSTER - create a new virtual cluster
// %Category: Experimental
// %Text:
// CREATE VIRTUAL CLUSTER [ IF NOT EXISTS ] name [ LIKE <virtual_cluster_spec> ] [ <replication> ]
// CREATE VIRTUAL CLUSTER [ IF NOT EXISTS ] name [ <replication> ]
//
// Replication option:
// FROM REPLICATION OF <virtual_cluster_spec> ON <location> [ WITH OPTIONS ... ]
create_virtual_cluster_stmt:
CREATE virtual_cluster d_expr opt_like_virtual_cluster
CREATE virtual_cluster d_expr
{
/* SKIP DOC */
$$.val = &tree.CreateTenant{
TenantSpec: &tree.TenantSpec{IsName: true, Expr: $3.expr()},
Like: $4.likeTenantSpec(),
}
}
| CREATE virtual_cluster IF NOT EXISTS d_expr opt_like_virtual_cluster
| CREATE virtual_cluster IF NOT EXISTS d_expr
{
/* SKIP DOC */
$$.val = &tree.CreateTenant{
IfNotExists: true,
TenantSpec: &tree.TenantSpec{IsName: true, Expr: $6.expr()},
Like: $7.likeTenantSpec(),
}
}
| CREATE virtual_cluster d_expr opt_like_virtual_cluster FROM REPLICATION OF d_expr ON d_expr opt_with_replication_options
| CREATE virtual_cluster d_expr FROM REPLICATION OF d_expr ON d_expr opt_with_replication_options
{
/* SKIP DOC */
$$.val = &tree.CreateTenantFromReplication{
TenantSpec: &tree.TenantSpec{IsName: true, Expr: $3.expr()},
ReplicationSourceTenantName: &tree.TenantSpec{IsName: true, Expr: $8.expr()},
ReplicationSourceAddress: $10.expr(),
Options: *$11.tenantReplicationOptions(),
Like: $4.likeTenantSpec(),
ReplicationSourceTenantName: &tree.TenantSpec{IsName: true, Expr: $7.expr()},
ReplicationSourceAddress: $9.expr(),
Options: *$10.tenantReplicationOptions(),
}
}
| CREATE virtual_cluster IF NOT EXISTS d_expr opt_like_virtual_cluster FROM REPLICATION OF d_expr ON d_expr opt_with_replication_options
| CREATE virtual_cluster IF NOT EXISTS d_expr FROM REPLICATION OF d_expr ON d_expr opt_with_replication_options
{
/* SKIP DOC */
$$.val = &tree.CreateTenantFromReplication{
IfNotExists: true,
TenantSpec: &tree.TenantSpec{IsName: true, Expr: $6.expr()},
ReplicationSourceTenantName: &tree.TenantSpec{IsName: true, Expr: $11.expr()},
ReplicationSourceAddress: $13.expr(),
Options: *$14.tenantReplicationOptions(),
Like: $7.likeTenantSpec(),
ReplicationSourceTenantName: &tree.TenantSpec{IsName: true, Expr: $10.expr()},
ReplicationSourceAddress: $12.expr(),
Options: *$13.tenantReplicationOptions(),
}
}
| CREATE virtual_cluster error // SHOW HELP: CREATE VIRTUAL CLUSTER
Expand All @@ -4795,19 +4787,6 @@ virtual_cluster:
TENANT { /* SKIP DOC */ }
| VIRTUAL CLUSTER

// opt_like_virtual_cluster defines a LIKE clause for CREATE VIRTUAL CLUSTER.
// Eventually this can grow to support INCLUDING/EXCLUDING options
// like in CREATE TABLE.
opt_like_virtual_cluster:
/* EMPTY */
{
$$.val = &tree.LikeTenantSpec{}
}
| LIKE virtual_cluster_spec
{
$$.val = &tree.LikeTenantSpec{OtherTenant: $2.tenantSpec()}
}

// Optional tenant replication options.
opt_with_replication_options:
WITH replication_options_list
Expand Down
Loading
Loading