From b5a7b7515adba6dcec6f067add28b4463f258420 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Mon, 21 Aug 2023 10:08:28 +0200 Subject: [PATCH] sql: fix another buglet with tenant settings This (hidden) bug was left over from #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. Release note: None --- .../testdata/logic_test/crdb_internal_tenant | 4 ++-- pkg/sql/crdb_internal.go | 4 +++- pkg/sql/delegate/show_all_cluster_settings.go | 12 ++++++------ pkg/sql/logictest/testdata/logic_test/crdb_internal | 4 ++-- .../testdata/logic_test/crdb_internal_catalog | 2 +- 5 files changed, 14 insertions(+), 12 deletions(-) diff --git a/pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant b/pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant index 6287522cc782..dcc6a71d0ca7 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant +++ b/pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant @@ -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 = '' diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go index 9c748f959694..ef4f6fc60fc9 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -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()) @@ -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 } diff --git a/pkg/sql/delegate/show_all_cluster_settings.go b/pkg/sql/delegate/show_all_cluster_settings.go index 6024648c9dc0..43e3d44fa457 100644 --- a/pkg/sql/delegate/show_all_cluster_settings.go +++ b/pkg/sql/delegate/show_all_cluster_settings.go @@ -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, @@ -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 `) } diff --git a/pkg/sql/logictest/testdata/logic_test/crdb_internal b/pkg/sql/logictest/testdata/logic_test/crdb_internal index ce8db9df7916..074751dfa4e5 100644 --- a/pkg/sql/logictest/testdata/logic_test/crdb_internal +++ b/pkg/sql/logictest/testdata/logic_test/crdb_internal @@ -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 = '' diff --git a/pkg/sql/logictest/testdata/logic_test/crdb_internal_catalog b/pkg/sql/logictest/testdata/logic_test/crdb_internal_catalog index 77f1b20bbbcf..80152358121a 100644 --- a/pkg/sql/logictest/testdata/logic_test/crdb_internal_catalog +++ b/pkg/sql/logictest/testdata/logic_test/crdb_internal_catalog @@ -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"}}