From 39d6de19893ceb1c5afb6b2c60a6284117079023 Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Wed, 3 Dec 2025 11:37:24 -0500 Subject: [PATCH] logictest: harden `onlyif config` and `skipif config` options Previously, if the specified config doesn't exist, we'd ignore it silently. For `skipif` directive this was somewhat harmless, but for `onlyif` we'd simply not execute the query if only a single (and incorrect) config was specified. This exposed a handful of typos as well as some leftovers for `local-mixed-25.3` and `local-schema-locked` configs that have been removed. Release note: None --- .../testdata/logic_test/read_committed | 2 +- .../testdata/logic_test/redact_descriptor | 308 ------------------ .../testdata/logic_test/schema_change_in_txn | 2 - pkg/sql/logictest/logic.go | 8 +- .../logictest/logictestbase/logictestbase.go | 19 ++ .../testdata/logic_test/alter_column_type | 10 - .../testdata/logic_test/alter_primary_key | 13 - .../testdata/logic_test/cluster_settings | 2 +- .../testdata/logic_test/create_index | 8 +- .../logictest/testdata/logic_test/sqlsmith | 1 - .../testdata/logic_test/udf_in_table | 14 - .../testdata/logic_test/vector_index | 2 +- 12 files changed, 32 insertions(+), 357 deletions(-) diff --git a/pkg/ccl/logictestccl/testdata/logic_test/read_committed b/pkg/ccl/logictestccl/testdata/logic_test/read_committed index 7b5b388ca1d8..24168ecd88df 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/read_committed +++ b/pkg/ccl/logictestccl/testdata/logic_test/read_committed @@ -1,4 +1,4 @@ -# LogicTest: !local-schema-locked !local-prepared +# LogicTest: !local-prepared subtest cursor diff --git a/pkg/ccl/logictestccl/testdata/logic_test/redact_descriptor b/pkg/ccl/logictestccl/testdata/logic_test/redact_descriptor index 3ac67956404a..30f6a1e8d207 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/redact_descriptor +++ b/pkg/ccl/logictestccl/testdata/logic_test/redact_descriptor @@ -46,7 +46,6 @@ CREATE VIEW redacted_descriptors AS # The legacy and declarative schema changer have slightly different implementation # of `ADD COLUMN ... DEFAULT ... CHECK(...)`, resulting in slightly different # descriptor. We will skip the config that uses legacy schema changer. -skipif config local-schema-locked skipif config local-legacy-schema-changer query T SELECT descriptor from redacted_descriptors where id = 'collate_partition'::REGCLASS; @@ -178,137 +177,6 @@ SELECT descriptor from redacted_descriptors where id = 'collate_partition'::REGC } } -onlyif config local-schema-locked -query T -SELECT descriptor from redacted_descriptors where id = 'collate_partition'::REGCLASS; ----- -{ - "table": { - "checks": [ - { - "columnIds": [ - 2 - ], - "constraintId": 2, - "expr": "b != '_':::STRING", - "name": "check_b" - } - ], - "columns": [ - { - "id": 1, - "name": "a", - "type": { - "family": "CollatedStringFamily", - "locale": "da", - "oid": 25 - } - }, - { - "defaultExpr": "'_':::STRING", - "id": 2, - "name": "b", - "nullable": true, - "type": { - "family": "StringFamily", - "oid": 25 - } - } - ], - "createAsOfTime": { - "wallTime": "0" - }, - "families": [ - { - "columnIds": [ - 1, - 2 - ], - "columnNames": [ - "a", - "b" - ], - "defaultColumnId": 2, - "name": "primary" - } - ], - "formatVersion": 3, - "id": 106, - "modificationTime": {}, - "name": "collate_partition", - "nextColumnId": 3, - "nextConstraintId": 5, - "nextFamilyId": 1, - "nextIndexId": 4, - "nextMutationId": 1, - "parentId": 104, - "primaryIndex": { - "compositeColumnIds": [ - 1 - ], - "constraintId": 3, - "createdExplicitly": true, - "encodingType": 1, - "foreignKey": {}, - "geoConfig": {}, - "id": 2, - "interleave": {}, - "keyColumnDirections": [ - "ASC" - ], - "keyColumnIds": [ - 1 - ], - "keyColumnNames": [ - "a" - ], - "name": "collate_partition_pkey", - "partitioning": { - "numColumns": 1, - "range": [ - { - "fromInclusive": "Xw==", - "name": "p1", - "toExclusive": "Xw==" - } - ] - }, - "sharded": {}, - "storeColumnIds": [ - 2 - ], - "storeColumnNames": [ - "b" - ], - "unique": true, - "vecConfig": {}, - "version": 4 - }, - "privileges": { - "ownerProto": "root", - "users": [ - { - "privileges": "2", - "userProto": "admin", - "withGrantOption": "2" - }, - { - "privileges": "2", - "userProto": "root", - "withGrantOption": "2" - } - ], - "version": 3 - }, - "replacementOf": { - "time": {} - }, - "schemaLocked": true, - "unexposedParentSchemaId": 105, - "version": "10" - } -} - statement ok CREATE TABLE subpartition ( a INT, b INT, c INT, @@ -331,182 +199,6 @@ CREATE TABLE subpartition ( PARTITION p3 VALUES IN (DEFAULT) ) -onlyif config local-schema-locked -query T -SELECT descriptor from redacted_descriptors where id = 'subpartition'::REGCLASS; ----- -{ - "table": { - "columns": [ - { - "id": 1, - "name": "a", - "type": { - "family": "IntFamily", - "oid": 20, - "width": 64 - } - }, - { - "id": 2, - "name": "b", - "type": { - "family": "IntFamily", - "oid": 20, - "width": 64 - } - }, - { - "id": 3, - "name": "c", - "nullable": true, - "type": { - "family": "IntFamily", - "oid": 20, - "width": 64 - } - } - ], - "createAsOfTime": {}, - "families": [ - { - "columnIds": [ - 1, - 2, - 3 - ], - "columnNames": [ - "a", - "b", - "c" - ], - "defaultColumnId": 3, - "name": "primary" - } - ], - "formatVersion": 3, - "id": 108, - "modificationTime": {}, - "name": "subpartition", - "nextColumnId": 4, - "nextConstraintId": 2, - "nextFamilyId": 1, - "nextIndexId": 2, - "nextMutationId": 1, - "parentId": 104, - "primaryIndex": { - "constraintId": 1, - "createdAtNanos": "0", - "encodingType": 1, - "foreignKey": {}, - "geoConfig": {}, - "id": 1, - "interleave": {}, - "keyColumnDirections": [ - "ASC", - "ASC" - ], - "keyColumnIds": [ - 1, - 2 - ], - "keyColumnNames": [ - "a", - "b" - ], - "name": "subpartition_pkey", - "partitioning": { - "list": [ - { - "name": "p1", - "subpartitioning": { - "list": [ - { - "name": "p1_1", - "subpartitioning": {}, - "values": [ - "Xw==" - ] - }, - { - "name": "p1_2", - "subpartitioning": {}, - "values": [ - "Xw==" - ] - } - ], - "numColumns": 1 - }, - "values": [ - "Xw==" - ] - }, - { - "name": "p2", - "subpartitioning": { - "list": [ - { - "name": "p2_1", - "subpartitioning": {}, - "values": [ - "Xw==" - ] - } - ], - "numColumns": 1 - }, - "values": [ - "Xw==" - ] - }, - { - "name": "p3", - "subpartitioning": {}, - "values": [ - "Xw==" - ] - } - ], - "numColumns": 1 - }, - "sharded": {}, - "storeColumnIds": [ - 3 - ], - "storeColumnNames": [ - "c" - ], - "unique": true, - "vecConfig": {}, - "version": 4 - }, - "privileges": { - "ownerProto": "root", - "users": [ - { - "privileges": "2", - "userProto": "admin", - "withGrantOption": "2" - }, - { - "privileges": "2", - "userProto": "root", - "withGrantOption": "2" - } - ], - "version": 3 - }, - "replacementOf": { - "time": {} - }, - "schemaLocked": true, - "unexposedParentSchemaId": 105, - "version": "1" - } -} - -skipif config local-schema-locked query T SELECT descriptor from redacted_descriptors where id = 'subpartition'::REGCLASS; ---- diff --git a/pkg/ccl/logictestccl/testdata/logic_test/schema_change_in_txn b/pkg/ccl/logictestccl/testdata/logic_test/schema_change_in_txn index aafcaa89c995..719bbe5607f9 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/schema_change_in_txn +++ b/pkg/ccl/logictestccl/testdata/logic_test/schema_change_in_txn @@ -1,5 +1,3 @@ -# LogicTest: !local-schema-locked - # Skip the rest of the test if a retry occurs. They can happen and are fine # but there's no way to encapsulate that in logictests. skip_on_retry diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go index c7a86a43f523..6ed0f26006f3 100644 --- a/pkg/sql/logictest/logic.go +++ b/pkg/sql/logictest/logic.go @@ -3381,7 +3381,9 @@ func (t *logicTest) processSubtest( for _, configName := range args { if t.cfg.Name == configName || logictestbase.ConfigIsInDefaultList(t.cfg.Name, configName) { s.SetSkip(fmt.Sprintf("unsupported configuration %s (%s)", configName, githubIssueStr(githubIssueID))) - break + } + if !logictestbase.ConfigExists(configName) { + return errors.Newf("logic test config %s doesn't exist", configName) } } case "backup-restore": @@ -3434,7 +3436,9 @@ func (t *logicTest) processSubtest( if t.cfg.Name == configName || logictestbase.ConfigIsInDefaultList(t.cfg.Name, configName) { // Our config matches one item in the list. shouldSkip = false - break + } + if !logictestbase.ConfigExists(configName) { + return errors.Newf("logic test config %s doesn't exist", configName) } } if shouldSkip { diff --git a/pkg/sql/logictest/logictestbase/logictestbase.go b/pkg/sql/logictest/logictestbase/logictestbase.go index 536cf4aac48a..c9e55079053a 100644 --- a/pkg/sql/logictest/logictestbase/logictestbase.go +++ b/pkg/sql/logictest/logictestbase/logictestbase.go @@ -808,6 +808,9 @@ func processConfigs( // config list. names := getDefaultConfigListNames(blockedConfig) if len(names) == 0 { + if !ConfigExists(blockedConfig) && blockedConfig != "metamorphic-batch-sizes" { + panic(fmt.Sprintf("attempted to block logic test config that doesn't exist: %s", blockedConfig)) + } blocklist[blockedConfig] = issueNo } else { for _, name := range names { @@ -900,6 +903,22 @@ func getDefaultConfigListNames(name string) []string { return DefaultConfigSets[name].ConfigNames() } +var allConfigNames = make(map[string]struct{}, len(LogicTestConfigs)) + +func init() { + for _, cfg := range LogicTestConfigs { + allConfigNames[cfg.Name] = struct{}{} + } +} + +// ConfigExists returns whether the given name matches either a config or an +// alias. +func ConfigExists(name string) bool { + _, config := allConfigNames[name] + _, alias := DefaultConfigSets[name] + return config || alias +} + // ConfigCalculator is used to enumerate a map of configuration -> file. type ConfigCalculator struct { ConfigOverrides, ConfigFilterOverrides []string diff --git a/pkg/sql/logictest/testdata/logic_test/alter_column_type b/pkg/sql/logictest/testdata/logic_test/alter_column_type index 5436d71323d6..112c414f48c2 100644 --- a/pkg/sql/logictest/testdata/logic_test/alter_column_type +++ b/pkg/sql/logictest/testdata/logic_test/alter_column_type @@ -1920,16 +1920,6 @@ t_droppedcol CREATE TABLE public.t_droppedcol ( CONSTRAINT t_droppedcol_pkey PRIMARY KEY (rowid ASC) ); -onlyif config local-schmea-locked -query TT -SHOW CREATE TABLE t_droppedcol; ----- -t_droppedcol CREATE TABLE public.t_droppedcol ( - dropme INT8 NULL, - rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), - CONSTRAINT t_droppedcol_pkey PRIMARY KEY (rowid ASC) - ) WITH (schema_locked = true); - statement ok DROP TABLE t_droppedcol; diff --git a/pkg/sql/logictest/testdata/logic_test/alter_primary_key b/pkg/sql/logictest/testdata/logic_test/alter_primary_key index cf78b153b6b8..883a33bfac26 100644 --- a/pkg/sql/logictest/testdata/logic_test/alter_primary_key +++ b/pkg/sql/logictest/testdata/logic_test/alter_primary_key @@ -1105,19 +1105,6 @@ t CREATE TABLE public.t ( FAMILY fam_0_x_y_rowid (x, y, rowid) ); -onlyif config local-schmema-locked -query TT -SHOW CREATE t ----- -t CREATE TABLE public.t ( - x INT8 NOT NULL, - y INT8 NULL, - rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(), - CONSTRAINT t_pkey PRIMARY KEY (x ASC), - INDEX t_y_idx (y ASC), - FAMILY fam_0_x_y_rowid (x, y, rowid) - ) WITH (schema_locked = true); - # Ensure that index y got rewritten. If it was not rewritten, # it would have an id less than 3. query IT diff --git a/pkg/sql/logictest/testdata/logic_test/cluster_settings b/pkg/sql/logictest/testdata/logic_test/cluster_settings index fbbc1d53df12..5a1be49285df 100644 --- a/pkg/sql/logictest/testdata/logic_test/cluster_settings +++ b/pkg/sql/logictest/testdata/logic_test/cluster_settings @@ -359,7 +359,7 @@ SHOW CLUSTER SETTING server.mem_profile.total_dump_size_limit FOR TENANT "cluste ---- NULL -onlyif config 3snode-tenant-default-config +onlyif config 3node-tenant-default-configs query error SHOW CLUSTER SETTINGS FOR TENANT can only be called by system operators SHOW CLUSTER SETTINGS FOR TENANT [10] diff --git a/pkg/sql/logictest/testdata/logic_test/create_index b/pkg/sql/logictest/testdata/logic_test/create_index index 23464b3ee422..f4703dee1a3e 100644 --- a/pkg/sql/logictest/testdata/logic_test/create_index +++ b/pkg/sql/logictest/testdata/logic_test/create_index @@ -724,11 +724,11 @@ SET CLUSTER SETTING bulkio.index_backfill.distributed_merge.mode = enabled # merge isn't implemented yet, so we get to the validation step and notice # we didn't ingest any rows. This is expected until the entire flow is # implemented. -skipif config local-mixed-25.3 local-mixed-25.4 +skipif config local-mixed-25.4 statement error pgcode XX000 pq: .*validation of non-unique index dist_merge_idx_idx failed: expected 3 rows, found 0.* CREATE INDEX dist_merge_idx_idx ON dist_merge_idx (b) -onlyif config local-mixed-25.3 local-mixed-25.4 +onlyif config local-mixed-25.4 statement error pq: .*distributed merge requires cluster version 26.* CREATE INDEX dist_merge_idx_idx ON dist_merge_idx (b) @@ -737,11 +737,11 @@ CREATE INDEX dist_merge_idx_idx ON dist_merge_idx (b) # TODO(158378): Declarative schema change don't yet implement the full flow for # distributed merge. So, we end up with a similar situation as above where we # get to validation and notice no rows were ingested. -skipif config local-mixed-25.3 local-mixed-25.4 +skipif config local-mixed-25.4 statement error pgcode 23505 pq: .*duplicate key value violates unique constraint.* ALTER TABLE dist_merge_idx ALTER PRIMARY KEY USING COLUMNS (b) -onlyif config local-mixed-25.3 local-mixed-25.4 +onlyif config local-mixed-25.4 statement error pq: .*distributed merge requires cluster version 26.* ALTER TABLE dist_merge_idx ALTER PRIMARY KEY USING COLUMNS (b) diff --git a/pkg/sql/logictest/testdata/logic_test/sqlsmith b/pkg/sql/logictest/testdata/logic_test/sqlsmith index d7df77170383..2ea6ccbfa08b 100644 --- a/pkg/sql/logictest/testdata/logic_test/sqlsmith +++ b/pkg/sql/logictest/testdata/logic_test/sqlsmith @@ -2,7 +2,6 @@ # Regression: #28836 (nulls in string_agg) -skipif config 3node-tenant-default-configs #72565 statement ok SELECT subq_0.c3 AS c0, subq_0.c6 AS c1, subq_0.c4 AS c2, CASE WHEN (SELECT start_key FROM crdb_internal.ranges LIMIT 1 OFFSET 6) < CAST(NULLIF(pg_catalog.string_agg(CAST((SELECT start_key FROM crdb_internal.ranges LIMIT 1 OFFSET 7) AS BYTES), CAST((SELECT pg_catalog.xor_agg(tgargs) FROM pg_catalog.pg_trigger) AS BYTES)) OVER (PARTITION BY subq_0.c0 ORDER BY subq_0.c0, subq_0.c5, subq_0.c2), CAST(NULL AS BYTES)) AS BYTES) THEN subq_0.c6 ELSE subq_0.c6 END AS c3, subq_0.c2 AS c4, subq_0.c7 AS c5, CAST(COALESCE(subq_0.c7, subq_0.c7) AS INT8) AS c6 FROM (SELECT ref_0.table_name AS c0, ref_0.table_catalog AS c1, ref_0.table_type AS c2, (SELECT rolcreatedb FROM pg_catalog.pg_roles LIMIT 1 OFFSET 79) AS c3, ref_0.table_name AS c4, ref_0.version AS c5, ref_0.version AS c6, ref_0.version AS c7 FROM information_schema.tables AS ref_0 WHERE (ref_0.version IS NOT NULL) OR (pg_catalog.set_masklen(CAST(CAST(NULL AS INET) AS INET), CAST(ref_0.version AS INT8)) != (SELECT pg_catalog.max(client_addr) FROM pg_catalog.pg_stat_activity)) LIMIT 101) AS subq_0 WHERE subq_0.c7 IS NOT NULL diff --git a/pkg/sql/logictest/testdata/logic_test/udf_in_table b/pkg/sql/logictest/testdata/logic_test/udf_in_table index 250fb43de0a0..253af10f2de9 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf_in_table +++ b/pkg/sql/logictest/testdata/logic_test/udf_in_table @@ -114,20 +114,6 @@ CREATE TABLE public.t1 ( FAMILY fam_0 (a, b, c, d, e) ) WITH (schema_locked = true); -onlyif config local-schema-locked -query T -SELECT create_statement FROM [SHOW CREATE TABLE t1]; ----- -CREATE TABLE public.t1 ( - a INT8 NOT NULL, - b INT8 NULL DEFAULT public.f1(), - c INT8 NULL, - d INT8 NULL ON UPDATE public.f1(), - e INT8 NULL, - CONSTRAINT t1_pkey PRIMARY KEY (a ASC), - FAMILY fam_0 (a, b, c, d, e) -); - # Make sure that back references are tracked properly. let $fn_id SELECT oid::int - 100000 FROM pg_catalog.pg_proc WHERE proname = 'f1'; diff --git a/pkg/sql/logictest/testdata/logic_test/vector_index b/pkg/sql/logictest/testdata/logic_test/vector_index index cc27f2258e31..9519daf66946 100644 --- a/pkg/sql/logictest/testdata/logic_test/vector_index +++ b/pkg/sql/logictest/testdata/logic_test/vector_index @@ -1,4 +1,4 @@ -# LogicTest: !local-schema-locked !local-prepared +# LogicTest: !local-prepared # knob-opt: allow-unsafe # TODO(andyk): Remove this once 25.3 is our minimum supported version.