Skip to content

Commit 0695f6a

Browse files
committed
sql: fix CREATE AS [SHOW CREATE FUNCTION] job failure
Fixes cockroachdb#106268 Previously `CREATE TABLE AS`/`CREATE MATERIALIZED VIEW AS` sourcing from `SHOW CREATE FUNCTION <function>` generated a failing schema change job with a `unknown function: f(): function undefined` error because the job runs in the system database and functions cannot be referenced cross database. This PR fixes this by using the original user session (which includes the database) in the schema change job. Release note (bug fix): Fixes the schema changer job when CREATE AS sources from SHOW CREATE FUNCTION: CREATE TABLE t AS SELECT * FROM [SHOW CREATE FUNCTION f];
1 parent 2ca6e52 commit 0695f6a

File tree

9 files changed

+90
-11
lines changed

9 files changed

+90
-11
lines changed

pkg/jobs/jobspb/jobs.proto

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -887,7 +887,9 @@ message SchemaChangeDetails {
887887
(gogoproto.customtype) = "github.com/cockroachdb/cockroach/pkg/util/uuid.UUID"
888888
];
889889

890-
// Next id 13.
890+
sessiondatapb.SessionData session_data = 13;
891+
892+
// Next id 14.
891893
}
892894

893895
message SchemaChangeProgress {

pkg/sql/create_as_test.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@ func TestCreateAsShow(t *testing.T) {
129129
testCases := []struct {
130130
sql string
131131
setup string
132-
skip bool
133132
}{
134133
{
135134
sql: "SHOW CLUSTER SETTINGS",
@@ -165,9 +164,6 @@ func TestCreateAsShow(t *testing.T) {
165164
{
166165
sql: "SHOW CREATE FUNCTION show_create_fn",
167166
setup: "CREATE FUNCTION show_create_fn(i int) RETURNS INT AS 'SELECT i' LANGUAGE SQL",
168-
// TODO(sql-foundations): Fix `unknown function: show_create_fn(): function undefined` error in job.
169-
// See https://github.com/cockroachdb/cockroach/issues/106268.
170-
skip: true,
171167
},
172168
{
173169
sql: "SHOW CREATE ALL TYPES",
@@ -294,9 +290,6 @@ func TestCreateAsShow(t *testing.T) {
294290

295291
for i, testCase := range testCases {
296292
t.Run(testCase.sql, func(t *testing.T) {
297-
if testCase.skip {
298-
return
299-
}
300293
if testCase.setup != "" {
301294
if s.StartedDefaultTestTenant() && strings.Contains(testCase.setup, "create_tenant") {
302295
// Only the system tenant has the ability to create other

pkg/sql/delegate/show_function.go

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,20 @@ FROM crdb_internal.create_function_statements
2727
WHERE schema_name = %[1]s
2828
AND function_name = %[2]s
2929
`
30-
un, ok := n.Name.FunctionReference.(*tree.UnresolvedName)
30+
resolvableFunctionReference := &n.Name
31+
un, ok := resolvableFunctionReference.FunctionReference.(*tree.UnresolvedName)
3132
if !ok {
3233
return nil, errors.AssertionFailedf("not a valid function name")
3334
}
3435

35-
fn, err := d.catalog.ResolveFunction(d.ctx, un, &d.evalCtx.SessionData().SearchPath)
36+
searchPath := &d.evalCtx.SessionData().SearchPath
37+
var fn *tree.ResolvedFunctionDefinition
38+
var err error
39+
if d.qualifyDataSourceNamesInAST {
40+
fn, err = resolvableFunctionReference.Resolve(d.ctx, searchPath, d.catalog)
41+
} else {
42+
fn, err = d.catalog.ResolveFunction(d.ctx, un, searchPath)
43+
}
3644
if err != nil {
3745
return nil, err
3846
}
@@ -41,12 +49,26 @@ AND function_name = %[2]s
4149
for _, o := range fn.Overloads {
4250
if o.IsUDF {
4351
udfSchema = o.Schema
52+
break
4453
}
4554
}
4655
if udfSchema == "" {
4756
return nil, errors.Errorf("function %s does not exist", tree.AsString(un))
4857
}
4958

59+
if d.qualifyDataSourceNamesInAST {
60+
referenceByName := resolvableFunctionReference.ReferenceByName
61+
if !referenceByName.HasExplicitSchema() {
62+
referenceByName.Parts[1] = udfSchema
63+
}
64+
if !referenceByName.HasExplicitCatalog() {
65+
referenceByName.Parts[2] = d.evalCtx.SessionData().Database
66+
}
67+
if referenceByName.NumParts < 3 {
68+
referenceByName.NumParts = 3
69+
}
70+
}
71+
5072
fullQuery := fmt.Sprintf(query, lexbase.EscapeSQLString(udfSchema), lexbase.EscapeSQLString(un.Parts[0]))
5173
return d.parse(fullQuery)
5274
}

pkg/sql/drop_function.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ func (p *planner) writeDropFuncSchemaChange(ctx context.Context, funcDesc *funcd
279279
DescriptorIDs: descpb.IDs{funcDesc.ID},
280280
Details: jobspb.SchemaChangeDetails{
281281
DroppedFunctions: descpb.IDs{funcDesc.ID},
282+
SessionData: &p.SessionData().SessionData,
282283
},
283284
Progress: jobspb.TypeSchemaChangeProgress{},
284285
}

pkg/sql/drop_schema.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ func (p *planner) createDropSchemaJob(
258258
// The version distinction for database jobs doesn't matter for jobs that
259259
// drop schemas.
260260
FormatVersion: jobspb.DatabaseJobFormatVersion,
261+
SessionData: &p.SessionData().SessionData,
261262
},
262263
Progress: jobspb.SchemaChangeProgress{},
263264
NonCancelable: true,

pkg/sql/logictest/testdata/logic_test/vectorize

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
# Note that there is no much benefit from running this file with other logic
44
# test configurations because we override vectorize setting.
55

6+
subtest todo1
7+
68
# NOTE: all queries in this file should run with vectorize=experimental_always
79
# unless they are known to be unsupported (like generate_series, etc). If you do
810
# need to execute an unsupported query, follow the pattern of
@@ -605,6 +607,14 @@ SELECT EXTRACT(YEAR FROM x) FROM extract_test
605607
----
606608
2017
607609

610+
statement ok
611+
RESET vectorize
612+
613+
subtest end
614+
615+
subtest 38937
616+
617+
608618
# Regression test for #38937
609619
statement ok
610620
CREATE TABLE t38937 (_int2) AS SELECT 1::INT2;
@@ -614,6 +624,13 @@ SELECT sum_int(_int2) FROM t38937
614624
----
615625
1
616626

627+
subtest end
628+
629+
subtest todo2
630+
631+
statement ok
632+
SET vectorize=experimental_always
633+
617634
# Regression tests for #38959
618635

619636
statement ok
@@ -1167,6 +1184,13 @@ SELECT crdb_internal_mvcc_timestamp IS NOT NULL FROM mvcc@i
11671184
----
11681185
true
11691186

1187+
statement ok
1188+
RESET vectorize
1189+
1190+
subtest end
1191+
1192+
subtest 51841
1193+
11701194
# Regression test for builtin funcs that take no arguments, combined with
11711195
# data that can't be safely decoded as input.
11721196

@@ -1176,6 +1200,10 @@ CREATE TABLE t51841 (a) AS SELECT gen_random_uuid();
11761200
statement ok
11771201
SELECT random() from t51841
11781202

1203+
subtest end
1204+
1205+
subtest todo3
1206+
11791207
# Regression test to ensure that we can EXPLAIN (VEC) plans that contain user
11801208
# defined types. We aren't interested in the output here, but are just ensuring
11811209
# that we don't panic.
@@ -1247,6 +1275,13 @@ SELECT b FROM t66706@u WHERE NOT (b = 'foo')
12471275
bar
12481276
bar
12491277

1278+
statement ok
1279+
RESET vectorize
1280+
1281+
subtest end
1282+
1283+
subtest 68040
1284+
12501285
# Regression test for ignoring the escaping in the LIKE pattern (#68040).
12511286
statement ok
12521287
CREATE TABLE t68040 (c) AS SELECT 'string with \ backslash'
@@ -1256,6 +1291,13 @@ SELECT c FROM t68040 WHERE c LIKE '%\\%'
12561291
----
12571292
string with \ backslash
12581293

1294+
subtest end
1295+
1296+
subtest todo4
1297+
1298+
statement ok
1299+
SET vectorize=experimental_always
1300+
12591301
# Regression test for #68979. The IN operator should evaluate correctly when the
12601302
# tuple contents are not sorted by the optimizer.
12611303
statement ok
@@ -1296,3 +1338,8 @@ query I
12961338
SELECT _int2 * _int2 FROM ints WHERE _int4 + _int4 = _int8 + 2
12971339
----
12981340
4
1341+
1342+
statement ok
1343+
RESET vectorize
1344+
1345+
subtest end

pkg/sql/schema.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ func (p *planner) writeSchemaDescChange(
7373
// The version distinction for database jobs doesn't matter for schema
7474
// jobs.
7575
FormatVersion: jobspb.DatabaseJobFormatVersion,
76+
SessionData: &p.SessionData().SessionData,
7677
},
7778
Progress: jobspb.SchemaChangeProgress{},
7879
NonCancelable: true,

pkg/sql/schema_changer.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import (
5151
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
5252
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
5353
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
54+
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
5455
"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
5556
"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
5657
"github.com/cockroachdb/cockroach/pkg/storage"
@@ -156,6 +157,7 @@ type SchemaChanger struct {
156157
clock *hlc.Clock
157158
settings *cluster.Settings
158159
execCfg *ExecutorConfig
160+
sessionData *sessiondatapb.SessionData
159161
}
160162

161163
// NewSchemaChangerForTesting only for tests.
@@ -305,6 +307,8 @@ func (sc *SchemaChanger) backfillQueryIntoTable(
305307
return err
306308
}
307309

310+
sd := NewInternalSessionData(ctx, sc.execCfg.Settings, "backfillQueryIntoTable")
311+
sd.SessionData = *sc.sessionData
308312
// Create an internal planner as the planner used to serve the user query
309313
// would have committed by this point.
310314
p, cleanup := NewInternalPlanner(
@@ -313,7 +317,7 @@ func (sc *SchemaChanger) backfillQueryIntoTable(
313317
username.RootUserName(),
314318
&MemoryMetrics{},
315319
sc.execCfg,
316-
NewInternalSessionData(ctx, sc.execCfg.Settings, "backfillQueryIntoTable"),
320+
sd,
317321
)
318322

319323
defer cleanup()
@@ -2155,6 +2159,7 @@ func (sc *SchemaChanger) updateJobForRollback(
21552159
TableMutationID: sc.mutationID,
21562160
ResumeSpanList: spanList,
21572161
FormatVersion: oldDetails.FormatVersion,
2162+
SessionData: sc.sessionData,
21582163
},
21592164
); err != nil {
21602165
return err
@@ -2611,6 +2616,7 @@ func (r schemaChangeResumer) Resume(ctx context.Context, execCtx interface{}) er
26112616
settings: p.ExecCfg().Settings,
26122617
execCfg: p.ExecCfg(),
26132618
metrics: p.ExecCfg().SchemaChangerMetrics,
2619+
sessionData: details.SessionData,
26142620
}
26152621
opts := retry.Options{
26162622
InitialBackoff: 20 * time.Millisecond,
@@ -2828,6 +2834,7 @@ func (r schemaChangeResumer) OnFailOrCancel(
28282834
clock: p.ExecCfg().Clock,
28292835
settings: p.ExecCfg().Settings,
28302836
execCfg: p.ExecCfg(),
2837+
sessionData: details.SessionData,
28312838
}
28322839

28332840
if r.job.Payload().FinalResumeError == nil {
@@ -2931,6 +2938,7 @@ func (sc *SchemaChanger) queueCleanupJob(
29312938
// The version distinction for database jobs doesn't matter for jobs on
29322939
// tables.
29332940
FormatVersion: jobspb.DatabaseJobFormatVersion,
2941+
SessionData: sc.sessionData,
29342942
},
29352943
Progress: jobspb.SchemaChangeProgress{},
29362944
NonCancelable: true,

pkg/sql/table.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ func (p *planner) createDropDatabaseJob(
7171
DroppedFunctions: funcIDs,
7272
DroppedDatabaseID: databaseID,
7373
FormatVersion: jobspb.DatabaseJobFormatVersion,
74+
SessionData: &p.SessionData().SessionData,
7475
},
7576
Progress: jobspb.SchemaChangeProgress{},
7677
NonCancelable: true,
@@ -92,6 +93,7 @@ func (p *planner) createNonDropDatabaseChangeJob(
9293
Details: jobspb.SchemaChangeDetails{
9394
DescID: databaseID,
9495
FormatVersion: jobspb.DatabaseJobFormatVersion,
96+
SessionData: &p.SessionData().SessionData,
9597
},
9698
Progress: jobspb.SchemaChangeProgress{},
9799
NonCancelable: true,
@@ -163,6 +165,7 @@ func (p *planner) createOrUpdateSchemaChangeJob(
163165
// The version distinction for database jobs doesn't matter for jobs on
164166
// tables.
165167
FormatVersion: jobspb.DatabaseJobFormatVersion,
168+
SessionData: &p.SessionData().SessionData,
166169
},
167170
Progress: jobspb.SchemaChangeProgress{},
168171
// Mark jobs without a mutation ID as non-cancellable,
@@ -193,6 +196,7 @@ func (p *planner) createOrUpdateSchemaChangeJob(
193196
// The version distinction for database jobs doesn't matter for jobs on
194197
// tables.
195198
FormatVersion: jobspb.DatabaseJobFormatVersion,
199+
SessionData: &p.SessionData().SessionData,
196200
}
197201
if oldDetails.TableMutationID != descpb.InvalidMutationID {
198202
// The previous queued schema change job was associated with a mutation,

0 commit comments

Comments
 (0)