diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index f52809ef91f3..1ef24c9d05a1 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -1838,6 +1838,10 @@ func dropColumnImpl( return droppedViews, validateDescriptor(params.ctx, params.p, tableDesc) } +// handleTTLStorageParamChange changes TTL storage parameters. descriptorChanged +// must be true if the descriptor was modified directly. The caller +// (alterTableNode), has a separate check to see if any mutations were +// enqueued. func handleTTLStorageParamChange( params runParams, tn *tree.TableName, tableDesc *tabledesc.Mutable, after *catpb.RowLevelTTL, ) (descriptorChanged bool, err error) { @@ -1957,15 +1961,16 @@ func handleTTLStorageParamChange( // Adding TTL requires adding the TTL job before adding the TTL fields. // Removing TTL requires removing the TTL job before removing the TTL fields. var direction descpb.DescriptorMutation_Direction + directlyModifiesDescriptor := false switch { case before == nil && after != nil: direction = descpb.DescriptorMutation_ADD case before != nil && after == nil: direction = descpb.DescriptorMutation_DROP default: - descriptorChanged = true + directlyModifiesDescriptor = true } - if !descriptorChanged { + if !directlyModifiesDescriptor { // Add TTL mutation so that job is scheduled in SchemaChanger. tableDesc.AddModifyRowLevelTTLMutation( &descpb.ModifyRowLevelTTL{RowLevelTTL: after}, @@ -1984,11 +1989,11 @@ func handleTTLStorageParamChange( } // Modify the TTL fields here because it will not be done in a mutation. - if descriptorChanged { + if directlyModifiesDescriptor { tableDesc.RowLevelTTL = after } - return descriptorChanged, nil + return directlyModifiesDescriptor, nil } // tryRemoveFKBackReferences determines whether the provided unique constraint diff --git a/pkg/sql/distsql_running.go b/pkg/sql/distsql_running.go index 13fd54a58ad9..e20f60589c56 100644 --- a/pkg/sql/distsql_running.go +++ b/pkg/sql/distsql_running.go @@ -901,6 +901,10 @@ func (dsp *DistSQLPlanner) Run( "unexpected concurrency for a flow that was forced to be planned locally")) return } + if buildutil.CrdbTestBuild && txn != nil && localState.MustUseLeafTxn() && flow.GetFlowCtx().Txn.Type() != kv.LeafTxn { + recv.SetError(errors.AssertionFailedf("unexpected root txn used when leaf txn expected")) + return + } noWait := planCtx.getPortalPauseInfo() != nil flow.Run(ctx, noWait) @@ -1253,7 +1257,9 @@ func (r *DistSQLReceiver) resetForLocalRerun(stats topLevelQueryStats) { r.closed = false r.stats = stats r.egressCounter = nil - atomic.StoreUint64(r.progressAtomic, math.Float64bits(0)) + if r.progressAtomic != nil { + atomic.StoreUint64(r.progressAtomic, math.Float64bits(0)) + } } // Release releases this DistSQLReceiver back to the pool. diff --git a/pkg/sql/logictest/testdata/logic_test/apply_join b/pkg/sql/logictest/testdata/logic_test/apply_join index 8f196f12980e..e28dbf8ad258 100644 --- a/pkg/sql/logictest/testdata/logic_test/apply_join +++ b/pkg/sql/logictest/testdata/logic_test/apply_join @@ -665,3 +665,74 @@ CREATE MATERIALIZED VIEW v1 AS UNION SELECT aid, pid FROM cte2 ); + +# Regression test for #111327 - the query shouldn't cause a nil-pointer error. +statement ok +CREATE TYPE greeting AS ENUM ('hello', 'howdy', 'hi', 'good day', 'morning'); + +statement ok +CREATE TABLE IF NOT EXISTS seed AS +SELECT + g :: INT2 AS _int2, + g :: INT4 AS _int4, + g :: INT8 AS _int8, + g :: FLOAT4 AS _float4, + g :: FLOAT8 AS _float8, + '2001-01-01' :: DATE + g AS _date, + '2001-01-01' :: TIMESTAMP + g * '1 day'::INTERVAL AS _timestamp, + '2001-01-01' :: TIMESTAMPTZ + g * '1 day'::INTERVAL AS _timestamptz, + g * '1 day' :: INTERVAL AS _interval, + g % 2 = 1 AS _bool, + g :: DECIMAL AS _decimal, + g :: STRING AS _string, + g :: STRING :: BYTES AS _bytes, + substring('00000000-0000-0000-0000-' || g :: STRING || '00000000000', 1, 36):: UUID AS _uuid, + '0.0.0.0' :: INET + g AS _inet, + g :: STRING :: JSONB AS _jsonb, + enum_range('hello' :: greeting) [g] as _enum +FROM + generate_series(1, 5) AS g; + +statement ok +INSERT INTO seed DEFAULT VALUES; + +statement ok +CREATE INDEX on seed (_int8, _float8, _date); + +statement ok +CREATE INVERTED INDEX on seed (_jsonb); + +skipif config local-mixed-23.1 +statement error pgcode 22P02 pq: pg_lsn\(\): invalid input syntax for type pg_lsn: \"1\" +SELECT + tab378984.crdb_internal_mvcc_timestamp AS "%pcol857759", + '48 years 7 mons 894 days 13:39:26.674765':::INTERVAL AS col857760, + tab378983.tableoid AS "coL857761", + tab378983.crdb_internal_mvcc_timestamp AS col857762, + (SELECT (-4999644074744333745):::INT8 AS "co""l857763" LIMIT 1:::INT8) AS col857764, + tab378983._inet AS col857765, + '2011-06-28 16:37:44.000635+00':::TIMESTAMPTZ AS col857766, + tab378984._bool AS col857767, + tab378983.tableoid AS col857768, + tab378984._timestamptz AS col857769, + NULL AS "Co😽l857770", + tab378984._string AS "c)ol857771", + NULL AS col857772, + tab378983._int4 AS c🙃ol857773, + tab378984._interval AS " col857774", + tab378984.crdb_internal_mvcc_timestamp AS "c%69ol'857775" +FROM + seed@seed__int8__float8__date_idx AS tab378983, + seed@[0] AS tab378984 +WHERE + ( + true + AND ( + '9E82DF40/BC8A8379':::PG_LSN::PG_LSN NOT IN ( + SELECT pg_lsn(tab378984._string::STRING)::PG_LSN::PG_LSN AS col857758 + FROM seed@[0] AS "%ptAb%v378985" WHERE "%ptAb%v378985"._bool LIMIT 22:::INT8 + ) + ) + ) +ORDER BY tab378983._timestamptz ASC NULLS FIRST +LIMIT 78:::INT8; diff --git a/pkg/sql/logictest/testdata/logic_test/pg_lsn b/pkg/sql/logictest/testdata/logic_test/pg_lsn index 40432b637482..de02a9047817 100644 --- a/pkg/sql/logictest/testdata/logic_test/pg_lsn +++ b/pkg/sql/logictest/testdata/logic_test/pg_lsn @@ -4,12 +4,6 @@ SELECT 'A01F0/1AAA'::pg_lsn ---- A01F0/1AAA -statement error could not parse pg_lsn -SELECT 'A/G'::pg_lsn - -statement error could not parse pg_lsn -SELECT '0G'::pg_lsn - statement ok CREATE TABLE pg_lsn_table(id pg_lsn PRIMARY KEY, val pg_lsn) @@ -80,3 +74,22 @@ SELECT * FROM ( VALUES 23021024706816 18446744073709551615 -18446744073709551615 + +statement error pgcode 22P02 pq: invalid input syntax for type pg_lsn: \"A/G\" +SELECT 'A/G'::pg_lsn + +statement error pgcode 22P02 pq: invalid input syntax for type pg_lsn: \"0G\" +SELECT '0G'::pg_lsn + +statement error pgcode 22P02 pq: invalid input syntax for type pg_lsn: \"ab\" +SELECT 'ab'::PG_LSN; + +statement error pgcode 22P02 pq: pg_lsn\(\): invalid input syntax for type pg_lsn: \"ab\" +SELECT pg_lsn('ab'); + +statement error pgcode 22P02 pq: invalid input syntax for type pg_lsn: \"1\" +SELECT '1'::PG_LSN; + +statement error pgcode 22P02 pq: invalid input syntax for type pg_lsn: \"\" +SELECT ''::PG_LSN; + diff --git a/pkg/sql/logictest/testdata/logic_test/procedure b/pkg/sql/logictest/testdata/logic_test/procedure index 7a04ebd4a5f0..7bb50647e1bd 100644 --- a/pkg/sql/logictest/testdata/logic_test/procedure +++ b/pkg/sql/logictest/testdata/logic_test/procedure @@ -46,6 +46,39 @@ SELECT currval('s') statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. SELECT p() +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +CALL p(p()) + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +SELECT * FROM (VALUES (1), (2), (3)) LIMIT p() + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +SELECT * FROM (VALUES (1), (2), (3)) ORDER BY p() + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +SELECT * FROM (VALUES (1), (2), (3)) v(i) WHERE i = p() + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +SELECT * FROM (VALUES (1), (2), (3)) v(i) GROUP BY i HAVING i > p() + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +SELECT * FROM (VALUES (1), (2)) v(i) JOIN (VALUES (2), (3)) w(j) ON i = p() + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +SELECT * FROM generate_series(1, p()) + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +SELECT * FROM generate_series(1, p()) + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +SELECT abs(p()) + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +SELECT nth_value(1, p()) OVER () FROM (VALUES (1), (2)) + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +SELECT nth_value(1, i) OVER (ORDER BY p()) FROM (VALUES (1), (2)) v(i) + statement ok CREATE OR REPLACE PROCEDURE p() LANGUAGE SQL AS '' @@ -85,6 +118,27 @@ CREATE OR REPLACE PROCEDURE p2() LANGUAGE SQL AS $$ CALL p(); $$ +statement error pgcode 42883 unknown function: p\(\) +CREATE FUNCTION err(i INT) RETURNS VOID LANGUAGE SQL AS 'SELECT p()' + +statement error pgcode 0A000 unimplemented: CALL usage inside a function definition +CREATE FUNCTION err(i INT) RETURNS VOID LANGUAGE SQL AS 'CALL p()' + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +CREATE TABLE err (i INT DEFAULT p()) + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +CREATE TABLE err (i INT AS (p()) STORED) + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +CREATE TABLE err (i INT, INDEX (p())) + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +CREATE TABLE err (a INT, b INT, INDEX (a, (b + p()))) + +statement error pgcode 42809 p\(\) is a procedure\nHINT: To call a procedure, use CALL. +CREATE TABLE err (a INT, INDEX (a) WHERE p() = 1) + statement ok CREATE TABLE t ( k INT PRIMARY KEY, @@ -108,6 +162,25 @@ SELECT * FROM t ---- 1 11 +statement ok +CREATE FUNCTION one() RETURNS INT LANGUAGE SQL AS 'SELECT 1' + +statement ok +CALL t_update(one(), 12) + +query II +SELECT * FROM t +---- +1 12 + +statement ok +CALL t_update(one(), one()+12) + +query II +SELECT * FROM t +---- +1 13 + statement ok CREATE FUNCTION t_update() RETURNS INT LANGUAGE SQL AS 'SELECT 1' diff --git a/pkg/sql/logictest/testdata/logic_test/select_for_update_read_committed b/pkg/sql/logictest/testdata/logic_test/select_for_update_read_committed index e8b73e4e66bd..d05be70db477 100644 --- a/pkg/sql/logictest/testdata/logic_test/select_for_update_read_committed +++ b/pkg/sql/logictest/testdata/logic_test/select_for_update_read_committed @@ -3,9 +3,6 @@ statement ok SET CLUSTER SETTING sql.txn.read_committed_syntax.enabled = true -statement ok -SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL READ COMMITTED - statement ok CREATE TABLE abc (a INT PRIMARY KEY, b INT, c INT, INDEX (b), FAMILY (a, b, c)) @@ -29,7 +26,7 @@ GRANT ALL on bcd TO testuser user testuser statement ok -BEGIN +BEGIN ISOLATION LEVEL READ COMMITTED query III rowsort SELECT * FROM abc WHERE a != 3 FOR UPDATE @@ -50,14 +47,18 @@ user root # Normal reads do not block. query III rowsort -SELECT * FROM abc +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT * FROM abc; +COMMIT; ---- 1 10 100 2 20 200 3 30 300 query III rowsort -SELECT * FROM bcd +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT * FROM bcd; +COMMIT; ---- 20 200 2000 30 300 3000 @@ -66,28 +67,34 @@ SELECT * FROM bcd # SKIP LOCKED reads do not block. query III rowsort -SELECT * FROM abc FOR UPDATE SKIP LOCKED +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT * FROM abc FOR UPDATE SKIP LOCKED; +COMMIT; ---- 3 30 300 query III rowsort -SELECT * FROM bcd FOR UPDATE SKIP LOCKED +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT * FROM bcd FOR UPDATE SKIP LOCKED; +COMMIT; ---- 20 200 2000 -30 300 3000 -40 400 4000 # Shared reads block on exclusive locks but not on shared locks. query III async,rowsort q00 -SELECT * FROM abc FOR SHARE +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT * FROM abc FOR SHARE; +COMMIT; ---- 1 11 101 2 21 201 3 30 300 query III rowsort -SELECT * FROM bcd FOR SHARE +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT * FROM bcd FOR SHARE; +COMMIT; ---- 20 200 2000 30 300 3000 @@ -96,14 +103,18 @@ SELECT * FROM bcd FOR SHARE # Exclusive reads block on both. query III async,rowsort q01 -SELECT * FROM abc FOR UPDATE +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT * FROM abc FOR UPDATE; +COMMIT; ---- 1 11 101 2 21 201 3 30 300 query III async,rowsort q02 -SELECT * FROM bcd FOR UPDATE +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT * FROM bcd FOR UPDATE; +COMMIT; ---- 20 200 2000 30 300 3000 @@ -112,28 +123,38 @@ SELECT * FROM bcd FOR UPDATE # Try more exclusive-locking queries. query I async q03 -SELECT a FROM abc WHERE a = 2 FOR UPDATE +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT a FROM abc WHERE a = 2 FOR UPDATE; +COMMIT; ---- 2 query I async q04 -SELECT b FROM abc WHERE a = 2 FOR UPDATE +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT b FROM abc WHERE a = 2 FOR UPDATE; +COMMIT; ---- 21 query I async q05 -SELECT c FROM abc WHERE a = 2 FOR UPDATE +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT c FROM abc WHERE a = 2 FOR UPDATE; +COMMIT; ---- 201 query I async q06 -SELECT c FROM abc ORDER BY a DESC LIMIT 2 FOR UPDATE +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT c FROM abc ORDER BY a DESC LIMIT 2 FOR UPDATE; +COMMIT; ---- 300 201 query I async,rowsort q07 -SELECT a + b + c FROM abc FOR UPDATE +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT a + b + c FROM abc FOR UPDATE; +COMMIT; ---- 113 224 @@ -141,13 +162,17 @@ SELECT a + b + c FROM abc FOR UPDATE # Try some joins -query IIIII async q08 -SELECT * FROM abc JOIN bcd USING (b) FOR SHARE +query IIIII async,rowsort q08 +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT * FROM abc JOIN bcd USING (b) FOR SHARE; +COMMIT; ---- 30 3 300 300 3000 -query IIIII async q09 -SELECT * FROM abc JOIN bcd USING (c) FOR UPDATE +query IIIII async,rowsort q09 +BEGIN ISOLATION LEVEL READ COMMITTED; +SELECT * FROM abc JOIN bcd USING (c) FOR UPDATE; +COMMIT; ---- 300 3 30 30 3000 diff --git a/pkg/sql/opt/invertedidx/json_array.go b/pkg/sql/opt/invertedidx/json_array.go index c7d4b39912e1..79b73662222d 100644 --- a/pkg/sql/opt/invertedidx/json_array.go +++ b/pkg/sql/opt/invertedidx/json_array.go @@ -568,7 +568,13 @@ func (j *jsonOrArrayFilterPlanner) extractJSONExistsCondition( func (j *jsonOrArrayFilterPlanner) extractJSONEqCondition( ctx context.Context, evalCtx *eval.Context, left *memo.VariableExpr, right opt.ScalarExpr, ) inverted.Expression { - // The right side of the expression should be a constant JSON value. + // The left side of the expression must be a variable expression of the + // indexed column. + if !isIndexColumn(j.tabID, j.index, left, j.computedColumns) { + return inverted.NonInvertedColExpression{} + } + + // The right side of the expression must be a constant JSON value. if !memo.CanExtractConstDatum(right) { return inverted.NonInvertedColExpression{} } @@ -577,8 +583,7 @@ func (j *jsonOrArrayFilterPlanner) extractJSONEqCondition( return inverted.NonInvertedColExpression{} } - // For Equals expressions, we will generate the inverted expression for the - // single object built from the keys and val. + // For Equals expressions, we will generate the inverted expression for val. invertedExpr := getInvertedExprForJSONOrArrayIndexForContaining(ctx, evalCtx, val) // Generated inverted expression won't be tight as we are searching for rows diff --git a/pkg/sql/opt/invertedidx/json_array_test.go b/pkg/sql/opt/invertedidx/json_array_test.go index 689934de715d..87eab23594ea 100644 --- a/pkg/sql/opt/invertedidx/json_array_test.go +++ b/pkg/sql/opt/invertedidx/json_array_test.go @@ -224,8 +224,16 @@ func TestTryFilterJsonOrArrayIndex(t *testing.T) { evalCtx := eval.NewTestingEvalContext(st) tc := testcat.New() - if _, err := tc.ExecuteDDL( - "CREATE TABLE t (j JSON, a INT[], str STRING[], INVERTED INDEX (j), INVERTED INDEX (a), INVERTED INDEX (str))", + if _, err := tc.ExecuteDDL(` + CREATE TABLE t ( + j JSON, + j2 JSON, + a INT[], + str STRING[], + INVERTED INDEX (j), + INVERTED INDEX (a), + INVERTED INDEX (str) + )`, ); err != nil { t.Fatal(err) } @@ -309,6 +317,12 @@ func TestTryFilterJsonOrArrayIndex(t *testing.T) { indexOrd: arrayOrd, ok: false, }, + { + // Filtering a non-indexed column. + filters: "j2 @> '1'", + indexOrd: jsonOrd, + ok: false, + }, { // When operations affecting two different variables are OR-ed, we cannot // constrain either index. @@ -468,6 +482,12 @@ func TestTryFilterJsonOrArrayIndex(t *testing.T) { unique: true, remainingFilters: "j->0 = '1'", }, + { + // Filtering a non-indexed column. + filters: "j2->0 = '1'", + indexOrd: jsonOrd, + ok: false, + }, { // Arrays on the right side of the equality are supported. filters: "j->'a' = '[1]'", @@ -594,6 +614,12 @@ func TestTryFilterJsonOrArrayIndex(t *testing.T) { unique: true, remainingFilters: `j->0 @> '{"b": "c"}'`, }, + { + // Filtering a non-indexed column. + filters: `j2->0 @> '{"b": "c"}'`, + indexOrd: jsonOrd, + ok: false, + }, { // The inner most expression is not a fetch val expression with an // indexed column on the left. @@ -791,6 +817,12 @@ func TestTryFilterJsonOrArrayIndex(t *testing.T) { unique: false, remainingFilters: "", }, + { + // Filtering a non-indexed column. + filters: `'1' <@ j2->'a'`, + indexOrd: jsonOrd, + ok: false, + }, { // JSONExists is supported. Unique is false for all Exists predicates // because they check containment within arrays as well. @@ -1080,6 +1112,12 @@ func TestTryFilterJsonOrArrayIndex(t *testing.T) { unique: false, remainingFilters: `j = '{"a": "b"}' OR j = '[1, 2, 3]'`, }, + { + // Filtering a non-indexed column. + filters: `j2 = '"a"'`, + indexOrd: jsonOrd, + ok: false, + }, { // Testing the IN operator without the fetch value operator. filters: `j IN ('1', '2', '3')`, @@ -1129,6 +1167,12 @@ func TestTryFilterJsonOrArrayIndex(t *testing.T) { unique: false, remainingFilters: `j IN ('[1, 2, 3]', '{"a": "b"}', '1', '"a"')`, }, + { + // Filtering a non-indexed column. + filters: `j2 IN ('1', '2', '3')`, + indexOrd: jsonOrd, + ok: false, + }, } for _, tc := range testCases { diff --git a/pkg/sql/opt/optbuilder/groupby.go b/pkg/sql/opt/optbuilder/groupby.go index 9c30f14be7f8..d035f55f827f 100644 --- a/pkg/sql/opt/optbuilder/groupby.go +++ b/pkg/sql/opt/optbuilder/groupby.go @@ -459,7 +459,8 @@ func (b *Builder) analyzeHaving(having *tree.Where, fromScope *scope) tree.Typed // in case we are recursively called within a subquery context. defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) b.semaCtx.Properties.Require( - exprKindHaving.String(), tree.RejectWindowApplications|tree.RejectGenerators, + exprKindHaving.String(), + tree.RejectWindowApplications|tree.RejectGenerators|tree.RejectProcedures, ) fromScope.context = exprKindHaving return fromScope.resolveAndRequireType(having.Expr, types.Bool) diff --git a/pkg/sql/opt/optbuilder/join.go b/pkg/sql/opt/optbuilder/join.go index 4e7fa5890e33..0bb0bb488808 100644 --- a/pkg/sql/opt/optbuilder/join.go +++ b/pkg/sql/opt/optbuilder/join.go @@ -117,7 +117,8 @@ func (b *Builder) buildJoin( if on, ok := cond.(*tree.OnJoinCond); ok { // Do not allow special functions in the ON clause. b.semaCtx.Properties.Require( - exprKindOn.String(), tree.RejectGenerators|tree.RejectWindowApplications, + exprKindOn.String(), + tree.RejectGenerators|tree.RejectWindowApplications|tree.RejectProcedures, ) outScope.context = exprKindOn filter := b.buildScalar( diff --git a/pkg/sql/opt/optbuilder/select.go b/pkg/sql/opt/optbuilder/select.go index 1d2cc235ee01..dfed05de054b 100644 --- a/pkg/sql/opt/optbuilder/select.go +++ b/pkg/sql/opt/optbuilder/select.go @@ -1332,7 +1332,7 @@ func (b *Builder) buildWhere(where *tree.Where, inScope *scope) { where.Expr, types.Bool, exprKindWhere, - tree.RejectGenerators|tree.RejectWindowApplications, + tree.RejectGenerators|tree.RejectWindowApplications|tree.RejectProcedures, inScope, ) diff --git a/pkg/sql/opt/optbuilder/srfs.go b/pkg/sql/opt/optbuilder/srfs.go index 9db0e3470c74..773880468926 100644 --- a/pkg/sql/opt/optbuilder/srfs.go +++ b/pkg/sql/opt/optbuilder/srfs.go @@ -89,7 +89,8 @@ func (b *Builder) buildZip(exprs tree.Exprs, inScope *scope) (outScope *scope) { // context. defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) b.semaCtx.Properties.Require(exprKindFrom.String(), - tree.RejectAggregates|tree.RejectWindowApplications|tree.RejectNestedGenerators) + tree.RejectAggregates|tree.RejectWindowApplications| + tree.RejectNestedGenerators|tree.RejectProcedures) inScope.context = exprKindFrom // Build each of the provided expressions. diff --git a/pkg/sql/opt/xform/testdata/rules/select b/pkg/sql/opt/xform/testdata/rules/select index 0d5aaf128d3e..9f6276be0ce1 100644 --- a/pkg/sql/opt/xform/testdata/rules/select +++ b/pkg/sql/opt/xform/testdata/rules/select @@ -8198,6 +8198,77 @@ select └── filters └── val:3 > st_maxdistance(geom:1, '010100000000000000000000000000000000000000') [outer=(1,3), immutable, constraints=(/3: (/NULL - ])] +# Regression test for #111963. Do not plan inverted index scans on columns that +# are not filtered in the query. +exec-ddl +CREATE TABLE t111963 ( + j1 JSON, + j2 JSON +) +---- + +exec-ddl +CREATE INVERTED INDEX idx111963 ON t111963 (j1) +---- + +opt expect-not=GenerateInvertedIndexScans +SELECT * FROM t111963 WHERE j2 = '1' +---- +select + ├── columns: j1:1 j2:2!null + ├── immutable + ├── fd: ()-->(2) + ├── scan t111963 + │ └── columns: j1:1 j2:2 + └── filters + └── j2:2 = '1' [outer=(2), immutable, constraints=(/2: [/'1' - /'1']; tight), fd=()-->(2)] + +opt expect-not=GenerateInvertedIndexScans +SELECT * FROM t111963 WHERE j2 IN ('1', '10', '100') +---- +select + ├── columns: j1:1 j2:2!null + ├── scan t111963 + │ └── columns: j1:1 j2:2 + └── filters + └── j2:2 IN ('1', '10', '100') [outer=(2), constraints=(/2: [/'1' - /'1'] [/'10' - /'10'] [/'100' - /'100']; tight)] + +exec-ddl +DROP INDEX idx111963 +---- + +exec-ddl +CREATE INVERTED INDEX idx111963 ON t111963 ((j1->'foo')) +---- + +opt expect-not=GenerateInvertedIndexScans +SELECT * FROM t111963 WHERE j1 = '1' +---- +select + ├── columns: j1:1!null j2:2 + ├── immutable + ├── fd: ()-->(1) + ├── scan t111963 + │ ├── columns: j1:1 j2:2 + │ └── computed column expressions + │ └── crdb_internal_idx_expr:7 + │ └── j1:1->'foo' + └── filters + └── j1:1 = '1' [outer=(1), immutable, constraints=(/1: [/'1' - /'1']; tight), fd=()-->(1)] + +opt expect-not=GenerateInvertedIndexScans +SELECT * FROM t111963 WHERE j1 IN ('1', '10', '100') +---- +select + ├── columns: j1:1!null j2:2 + ├── scan t111963 + │ ├── columns: j1:1 j2:2 + │ └── computed column expressions + │ └── crdb_internal_idx_expr:7 + │ └── j1:1->'foo' + └── filters + └── j1:1 IN ('1', '10', '100') [outer=(1), constraints=(/1: [/'1' - /'1'] [/'10' - /'10'] [/'100' - /'100']; tight)] + # -------------------------------------------------- # GenerateZigzagJoins # -------------------------------------------------- diff --git a/pkg/sql/randgen/schema.go b/pkg/sql/randgen/schema.go index 234c6518b418..d47f13365684 100644 --- a/pkg/sql/randgen/schema.go +++ b/pkg/sql/randgen/schema.go @@ -517,7 +517,28 @@ func randIndexTableDefFromCols( cpy := make([]*tree.ColumnTableDef, len(columnTableDefs)) copy(cpy, columnTableDefs) rng.Shuffle(len(cpy), func(i, j int) { cpy[i], cpy[j] = cpy[j], cpy[i] }) - nCols := rng.Intn(len(cpy)) + 1 + + // Determine the number of indexed columns. + var nCols int + r := rng.Intn(100) + switch { + case r < 50: + // Create a single-column index 40% of the time. Single-column indexes + // are more likely then multi-column indexes to be used in query plans + // for randomly generated queries, so there is some benefit to + // guaranteeing that they are generated often. + nCols = 1 + case r < 75: + nCols = 2 + case r < 90: + nCols = 3 + default: + nCols = rng.Intn(len(cpy)) + 1 + } + if nCols > len(cpy) { + // nCols cannot be greater than the length of columnTableDefs. + nCols = len(cpy) + } cols := cpy[:nCols] diff --git a/pkg/sql/sem/tree/constant_test.go b/pkg/sql/sem/tree/constant_test.go index 714f63264330..c028421efcf4 100644 --- a/pkg/sql/sem/tree/constant_test.go +++ b/pkg/sql/sem/tree/constant_test.go @@ -221,7 +221,8 @@ func TestStringConstantVerifyAvailableTypes(t *testing.T) { semaCtx := tree.MakeSemaContext() if _, err := test.c.ResolveAsType(context.Background(), &semaCtx, availType); err != nil { - if !strings.Contains(err.Error(), "could not parse") { + if !strings.Contains(err.Error(), "could not parse") && + !strings.Contains(err.Error(), "invalid input syntax") { // Parsing errors are permitted for this test, as proper tree.StrVal parsing // is tested in TestStringConstantTypeResolution. Any other error should // throw a failure. @@ -693,7 +694,8 @@ func TestStringConstantResolveAvailableTypes(t *testing.T) { if !strings.Contains(err.Error(), "could not parse") && !strings.Contains(err.Error(), "parsing") && !strings.Contains(err.Error(), "out of range") && - !strings.Contains(err.Error(), "exceeds supported") { + !strings.Contains(err.Error(), "exceeds supported") && + !strings.Contains(err.Error(), "invalid input syntax") { // Parsing errors are permitted for this test, but the number of correctly // parseable types will be verified. Any other error should throw a failure. t.Errorf("%d: expected resolving %v as available type %s would either succeed"+ diff --git a/pkg/sql/sem/tree/datum.go b/pkg/sql/sem/tree/datum.go index e3b9e53d68fb..74247a6105f3 100644 --- a/pkg/sql/sem/tree/datum.go +++ b/pkg/sql/sem/tree/datum.go @@ -3629,7 +3629,9 @@ func NewDPGLSN(l lsn.LSN) *DPGLSN { func ParseDPGLSN(str string) (*DPGLSN, error) { v, err := lsn.ParseLSN(str) if err != nil { - return nil, errors.Wrapf(err, "could not parse pg_lsn") + return nil, pgerror.Newf(pgcode.InvalidTextRepresentation, + "invalid input syntax for type pg_lsn: \"%s\"", str, + ) } return NewDPGLSN(v), nil } diff --git a/pkg/sql/sem/tree/type_check.go b/pkg/sql/sem/tree/type_check.go index ca16911b703f..5def848679c2 100644 --- a/pkg/sql/sem/tree/type_check.go +++ b/pkg/sql/sem/tree/type_check.go @@ -120,6 +120,11 @@ func (s *SemaProperties) Require(context string, rejectFlags SemaRejectFlags) { s.Ancestors.clear() } +// Reject adds the given flags to the set of required constraints of s. +func (s *SemaProperties) Reject(rejectFlags SemaRejectFlags) { + s.required.rejectFlags |= rejectFlags +} + // IsSet checks if the given rejectFlag is set as a required property. func (s *SemaProperties) IsSet(rejectFlags SemaRejectFlags) bool { return s.required.rejectFlags&rejectFlags != 0 @@ -171,8 +176,14 @@ const ( // RejectSubqueries rejects subqueries in scalar contexts. RejectSubqueries + // RejectProcedures rejects procedures in scalar contexts. + RejectProcedures + // RejectSpecial is used in common places like the LIMIT clause. - RejectSpecial = RejectAggregates | RejectGenerators | RejectWindowApplications + RejectSpecial = RejectAggregates | + RejectGenerators | + RejectWindowApplications | + RejectProcedures ) // ScalarProperties contains the properties of the current scalar @@ -1156,8 +1167,19 @@ func (expr *FuncExpr) TypeCheck( (*qualifiedOverloads)(&def.Overloads), expr.Exprs..., ) defer s.release() - if err := s.typeCheckOverloadedExprs(ctx, semaCtx, desired, false); err != nil { - return nil, pgerror.Wrapf(err, pgcode.InvalidParameterValue, "%s()", def.Name) + + if err := func() error { + // Disallow procedures in function arguments. + if semaCtx != nil { + defer semaCtx.Properties.Restore(semaCtx.Properties) + semaCtx.Properties.Reject(RejectProcedures) + } + if err := s.typeCheckOverloadedExprs(ctx, semaCtx, desired, false); err != nil { + return pgerror.Wrapf(err, pgcode.InvalidParameterValue, "%s()", def.Name) + } + return nil + }(); err != nil { + return nil, err } var hasUDFOverload bool @@ -1267,6 +1289,16 @@ func (expr *FuncExpr) TypeCheck( } } + if overloadImpl.Type == ProcedureRoutine && semaCtx.Properties.IsSet(RejectProcedures) { + return nil, errors.WithHint( + pgerror.Newf( + pgcode.WrongObjectType, + "%s(%s) is a procedure", def.Name, overloadImpl.Types.String(), + ), + "To call a procedure, use CALL.", + ) + } + if expr.IsWindowFunctionApplication() { // Make sure the window function application is of either a built-in window // function or of a builtin aggregate function. diff --git a/pkg/util/log/clog_test.go b/pkg/util/log/clog_test.go index f6af4b89bf3f..870b83bb6b2d 100644 --- a/pkg/util/log/clog_test.go +++ b/pkg/util/log/clog_test.go @@ -586,17 +586,23 @@ func TestFatalStacktraceStderr(t *testing.T) { if !strings.Contains(cont, "clog_test") { t.Fatalf("stack trace does not contain file name: %s", cont) } + + // NB: the string "!goroutine" is used here in order to match the + // goroutine headers in the formatted output. The stacktrace + // itself can sometimes contain the string `goroutine` if one + // goroutine is spawned from another due to + // https://github.com/golang/go/commit/51225f6fc648ba3e833f3493700c2996a816bdaa switch traceback { case tracebackNone: - if strings.Count(cont, "goroutine ") > 0 { + if strings.Count(cont, "!goroutine ") > 0 { t.Fatalf("unexpected stack trace:\n%s", cont) } case tracebackSingle: - if strings.Count(cont, "goroutine ") != 1 { + if strings.Count(cont, "!goroutine ") != 1 { t.Fatalf("stack trace contains too many goroutines: %s", cont) } case tracebackAll: - if strings.Count(cont, "goroutine ") < 2 { + if strings.Count(cont, "!goroutine ") < 2 { t.Fatalf("stack trace contains less than two goroutines: %s", cont) } }