Skip to content

Commit

Permalink
Merge cockroachdb#109118
Browse files Browse the repository at this point in the history 
109118: sql: fix another buglet with tenant settings r=stevendanna a=knz

This (hidden) bug was left over from cockroachdb#108902.
There is no visible negative effect to this bug being present until setting aliases are effectively introduced, which hasn't happened yet.

The code modified here is already properly exercised by logic tests.
Also, this is exercised by cockroachdb#109077 and cockroachdb#109074 and the CI in those PRs readily fail without this fix.

Epic: CRDB-27642

Co-authored-by: Raphael 'kena' Poss <[email protected]>
  • Loading branch information
@knz
craig[bot] and knz committed Aug 21, 2023
2 parents 0ebbd52 + b5a7b75 commit 9ad8453
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 12 deletions.
4 changes: 2 additions & 2 deletions pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,10 @@ SELECT * FROM crdb_internal.session_trace WHERE span_idx < 0
----
span_idx message_idx timestamp duration operation loc tag message age

query TTTBTTT colnames
query TTTBTTTT colnames
SELECT * FROM crdb_internal.cluster_settings WHERE variable = ''
----
variable value type public description default_value origin
variable value type public description default_value origin key

query TI colnames
SELECT * FROM crdb_internal.feature_usage WHERE feature_name = ''
Expand Down
4 changes: 3 additions & 1 deletion pkg/sql/crdb_internal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2336,7 +2336,8 @@ CREATE TABLE crdb_internal.cluster_settings (
public BOOL NOT NULL, -- whether the setting is documented, which implies the user can expect support.
description STRING NOT NULL,
default_value STRING NOT NULL,
origin STRING NOT NULL -- the origin of the value: 'default' , 'override' or 'external-override'
origin STRING NOT NULL, -- the origin of the value: 'default' , 'override' or 'external-override'
key STRING NOT NULL
)`,
populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error {
hasSqlModify, err := p.HasPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.MODIFYSQLCLUSTERSETTING, p.User())
Expand Down Expand Up @@ -2388,6 +2389,7 @@ CREATE TABLE crdb_internal.cluster_settings (
tree.NewDString(desc),
tree.NewDString(defaultVal),
tree.NewDString(origin),
tree.NewDString(string(setting.InternalKey())),
); err != nil {
return err
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/sql/delegate/show_all_cluster_settings.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,17 +131,17 @@ WITH
LEFT JOIN system.tenants st ON id = tenant_id.tenant_id
),
tenantspecific AS (
SELECT t.name, t.value
SELECT t.name AS setting_key, t.value
FROM system.tenant_settings t, tenant_id
WHERE t.tenant_id = tenant_id.tenant_id
),
allsettings AS (
SELECT variable, value, public, type, description
SELECT variable AS setting_name, value, public, type, description, key AS setting_key
FROM system.crdb_internal.cluster_settings ` + publicFilter + `
)
SELECT
allsettings.variable || substr('', (SELECT ok FROM isvalid)) AS variable,
crdb_internal.decode_cluster_setting(allsettings.variable,
allsettings.setting_name || substr('', (SELECT ok FROM isvalid)) AS variable,
crdb_internal.decode_cluster_setting(allsettings.setting_name,
-- NB: careful not to coalesce with allsettings.value directly!
-- This is the value for the system tenant and is not relevant to other tenants.
COALESCE(tenantspecific.value,
Expand All @@ -161,8 +161,8 @@ SELECT
FROM
allsettings
LEFT JOIN tenantspecific ON
allsettings.variable = tenantspecific.name
allsettings.setting_key = tenantspecific.setting_key
LEFT JOIN system.tenant_settings AS overrideall ON
allsettings.variable = overrideall.name AND overrideall.tenant_id = 0
allsettings.setting_key = overrideall.name AND overrideall.tenant_id = 0
`)
}
4 changes: 2 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/crdb_internal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,10 @@ SELECT * FROM crdb_internal.session_trace WHERE span_idx < 0
----
span_idx message_idx timestamp duration operation loc tag message age

query TTTBTTT colnames
query TTTBTTTT colnames
SELECT * FROM crdb_internal.cluster_settings WHERE variable = ''
----
variable value type public description default_value origin
variable value type public description default_value origin key

query TI colnames
SELECT * FROM crdb_internal.feature_usage WHERE feature_name = ''
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/crdb_internal_catalog
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ SELECT id, strip_volatile(descriptor) FROM crdb_internal.kv_catalog_descriptor O
4294967271 {"table": {"columns": [{"id": 1, "name": "database_id", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 2, "name": "database_name", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 3, "name": "schema_id", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 4, "name": "schema_name", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 5, "name": "function_id", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 6, "name": "function_name", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 7, "name": "create_statement", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}], "formatVersion": 3, "id": 4294967271, "name": "create_function_statements", "nextColumnId": 8, "nextConstraintId": 2, "nextIndexId": 2, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 2}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1"}}
4294967272 {"table": {"columns": [{"id": 1, "name": "aggregated_ts", "type": {"family": "TimestampTZFamily", "oid": 1184}}, {"id": 2, "name": "fingerprint_id", "type": {"family": "BytesFamily", "oid": 17}}, {"id": 3, "name": "app_name", "type": {"family": "StringFamily", "oid": 25}}, {"id": 4, "name": "metadata", "type": {"family": "JsonFamily", "oid": 3802}}, {"id": 5, "name": "statistics", "type": {"family": "JsonFamily", "oid": 3802}}, {"id": 6, "name": "aggregation_interval", "type": {"family": "IntervalFamily", "intervalDurationField": {}, "oid": 1186}}], "formatVersion": 3, "id": 4294967272, "name": "cluster_transaction_statistics", "nextColumnId": 7, "nextConstraintId": 2, "nextIndexId": 2, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 2}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1"}}
4294967273 {"table": {"columns": [{"id": 1, "name": "aggregated_ts", "type": {"family": "TimestampTZFamily", "oid": 1184}}, {"id": 2, "name": "fingerprint_id", "type": {"family": "BytesFamily", "oid": 17}}, {"id": 3, "name": "transaction_fingerprint_id", "type": {"family": "BytesFamily", "oid": 17}}, {"id": 4, "name": "plan_hash", "type": {"family": "BytesFamily", "oid": 17}}, {"id": 5, "name": "app_name", "type": {"family": "StringFamily", "oid": 25}}, {"id": 6, "name": "metadata", "type": {"family": "JsonFamily", "oid": 3802}}, {"id": 7, "name": "statistics", "type": {"family": "JsonFamily", "oid": 3802}}, {"id": 8, "name": "sampled_plan", "type": {"family": "JsonFamily", "oid": 3802}}, {"id": 9, "name": "aggregation_interval", "type": {"family": "IntervalFamily", "intervalDurationField": {}, "oid": 1186}}, {"id": 10, "name": "index_recommendations", "type": {"arrayContents": {"family": "StringFamily", "oid": 25}, "arrayElemType": "StringFamily", "family": "ArrayFamily", "oid": 1009}}], "formatVersion": 3, "id": 4294967273, "name": "cluster_statement_statistics", "nextColumnId": 11, "nextConstraintId": 2, "nextIndexId": 2, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 2}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1"}}
4294967274 {"table": {"columns": [{"id": 1, "name": "variable", "type": {"family": "StringFamily", "oid": 25}}, {"id": 2, "name": "value", "type": {"family": "StringFamily", "oid": 25}}, {"id": 3, "name": "type", "type": {"family": "StringFamily", "oid": 25}}, {"id": 4, "name": "public", "type": {"oid": 16}}, {"id": 5, "name": "description", "type": {"family": "StringFamily", "oid": 25}}, {"id": 6, "name": "default_value", "type": {"family": "StringFamily", "oid": 25}}, {"id": 7, "name": "origin", "type": {"family": "StringFamily", "oid": 25}}], "formatVersion": 3, "id": 4294967274, "name": "cluster_settings", "nextColumnId": 8, "nextConstraintId": 2, "nextIndexId": 2, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 2}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1"}}
4294967274 {"table": {"columns": [{"id": 1, "name": "variable", "type": {"family": "StringFamily", "oid": 25}}, {"id": 2, "name": "value", "type": {"family": "StringFamily", "oid": 25}}, {"id": 3, "name": "type", "type": {"family": "StringFamily", "oid": 25}}, {"id": 4, "name": "public", "type": {"oid": 16}}, {"id": 5, "name": "description", "type": {"family": "StringFamily", "oid": 25}}, {"id": 6, "name": "default_value", "type": {"family": "StringFamily", "oid": 25}}, {"id": 7, "name": "origin", "type": {"family": "StringFamily", "oid": 25}}, {"id": 8, "name": "key", "type": {"family": "StringFamily", "oid": 25}}], "formatVersion": 3, "id": 4294967274, "name": "cluster_settings", "nextColumnId": 9, "nextConstraintId": 2, "nextIndexId": 2, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 2}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1"}}
4294967275 {"table": {"columns": [{"id": 1, "name": "node_id", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 2, "name": "session_id", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 3, "name": "user_name", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 4, "name": "client_address", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 5, "name": "application_name", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 6, "name": "active_queries", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 7, "name": "last_active_query", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 8, "name": "num_txns_executed", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 9, "name": "session_start", "nullable": true, "type": {"family": "TimestampTZFamily", "oid": 1184}}, {"id": 10, "name": "active_query_start", "nullable": true, "type": {"family": "TimestampTZFamily", "oid": 1184}}, {"id": 11, "name": "kv_txn", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 12, "name": "alloc_bytes", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 13, "name": "max_alloc_bytes", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 14, "name": "status", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 15, "name": "session_end", "nullable": true, "type": {"family": "TimestampTZFamily", "oid": 1184}}], "formatVersion": 3, "id": 4294967275, "name": "cluster_sessions", "nextColumnId": 16, "nextConstraintId": 2, "nextIndexId": 2, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 2}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1"}}
4294967276 {"table": {"columns": [{"id": 1, "name": "id", "nullable": true, "type": {"family": "UuidFamily", "oid": 2950}}, {"id": 2, "name": "node_id", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 3, "name": "session_id", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 4, "name": "start", "nullable": true, "type": {"family": "TimestampFamily", "oid": 1114}}, {"id": 5, "name": "txn_string", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 6, "name": "application_name", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 7, "name": "num_stmts", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 8, "name": "num_retries", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 9, "name": "num_auto_retries", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 10, "name": "last_auto_retry_reason", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 11, "name": "isolation_level", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 12, "name": "priority", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 13, "name": "quality_of_service", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}], "formatVersion": 3, "id": 4294967276, "name": "cluster_transactions", "nextColumnId": 14, "nextConstraintId": 2, "nextIndexId": 2, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 2}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1"}}
4294967277 {"table": {"columns": [{"id": 1, "name": "query_id", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 2, "name": "txn_id", "nullable": true, "type": {"family": "UuidFamily", "oid": 2950}}, {"id": 3, "name": "node_id", "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 4, "name": "session_id", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 5, "name": "user_name", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 6, "name": "start", "nullable": true, "type": {"family": "TimestampTZFamily", "oid": 1184}}, {"id": 7, "name": "query", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 8, "name": "client_address", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 9, "name": "application_name", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 10, "name": "distributed", "nullable": true, "type": {"oid": 16}}, {"id": 11, "name": "phase", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 12, "name": "full_scan", "nullable": true, "type": {"oid": 16}}, {"id": 13, "name": "plan_gist", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"id": 14, "name": "database", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}], "formatVersion": 3, "id": 4294967277, "name": "cluster_queries", "nextColumnId": 15, "nextConstraintId": 2, "nextIndexId": 2, "nextMutationId": 1, "primaryIndex": {"constraintId": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "partitioning": {}, "sharded": {}}, "privileges": {"ownerProto": "node", "users": [{"privileges": "32", "userProto": "public"}], "version": 2}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 4294967295, "version": "1"}}
Expand Down

0 comments on commit 9ad8453

Please sign in to comment.