From 41e5e6abd7577d9dc0092c9da286dc7fbe152596 Mon Sep 17 00:00:00 2001 From: Chengxiong Ruan Date: Wed, 1 Feb 2023 23:37:38 -0500 Subject: [PATCH] sql: implement function OID reference This commit implements `[FUNCTION xxxx]` OID references syntax of functions. With this change, functions can be called with the new OID numerical representation. For example `SELECT [FUNCTION 123]('helloworld')`. The intention of this syntax is only for internal serialization of references to UDFs. But it's general enough for builtin functions as well. A new implementation of the `ResolvableFunctionReference` interface, `OIDFunctionReference` is introduced for function resolution purpose for the new syntax. The `ResolveFunctionByOID` method is also refactored to return a qualified name. Fixes: #83231 Release note: None --- docs/generated/sql/bnf/stmt_block.bnf | 30 ++-- .../changefeedccl/cdceval/func_resolver.go | 8 +- .../tests/3node-tenant/generated_test.go | 7 + pkg/sql/faketreeeval/evalctx.go | 4 +- pkg/sql/importer/import_table_creation.go | 4 +- .../logictest/testdata/logic_test/udf_oid_ref | 134 ++++++++++++++++++ .../tests/fakedist-disk/generated_test.go | 7 + .../tests/fakedist-vec-off/generated_test.go | 7 + .../tests/fakedist/generated_test.go | 7 + .../generated_test.go | 7 + .../tests/local-vec-off/generated_test.go | 7 + .../logictest/tests/local/generated_test.go | 7 + pkg/sql/opt/cat/catalog.go | 2 +- pkg/sql/opt/testutils/testcat/function.go | 4 +- pkg/sql/opt_catalog.go | 2 +- pkg/sql/parser/sql.y | 45 ++++-- .../parser/testdata/select_numeric_func_expr | 55 +++++++ pkg/sql/pg_catalog.go | 2 +- pkg/sql/schema_resolver.go | 29 ++-- pkg/sql/schemachanger/scdeps/build_deps.go | 2 +- .../scdeps/sctestdeps/test_deps.go | 32 +++-- pkg/sql/sem/builtins/BUILD.bazel | 1 + pkg/sql/sem/builtins/all_builtins.go | 28 ++-- pkg/sql/sem/builtins/all_builtins_test.go | 47 ++++-- pkg/sql/sem/eval/cast.go | 2 +- pkg/sql/sem/tree/function_definition.go | 17 +++ pkg/sql/sem/tree/function_name.go | 29 +++- pkg/sql/sem/tree/type_check.go | 25 +++- 28 files changed, 454 insertions(+), 97 deletions(-) create mode 100644 pkg/sql/logictest/testdata/logic_test/udf_oid_ref create mode 100644 pkg/sql/parser/testdata/select_numeric_func_expr diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf index a091739d23f0..616da8612116 100644 --- a/docs/generated/sql/bnf/stmt_block.bnf +++ b/docs/generated/sql/bnf/stmt_block.bnf @@ -3495,11 +3495,11 @@ opt_interval_qualifier ::= | func_application ::= - func_name '(' ')' - | func_name '(' expr_list opt_sort_clause ')' - | func_name '(' 'ALL' expr_list opt_sort_clause ')' - | func_name '(' 'DISTINCT' expr_list ')' - | func_name '(' '*' ')' + func_application_name '(' ')' + | func_application_name '(' expr_list opt_sort_clause ')' + | func_application_name '(' 'ALL' expr_list opt_sort_clause ')' + | func_application_name '(' 'DISTINCT' expr_list ')' + | func_application_name '(' '*' ')' within_group_clause ::= 'WITHIN' 'GROUP' '(' single_sort_clause ')' @@ -3751,10 +3751,9 @@ rowsfrom_item ::= opt_col_def_list ::= '(' col_def_list ')' -func_name ::= - type_function_name - | prefixed_column_path - | 'INDEX' +func_application_name ::= + func_name + | '[' 'FUNCTION' iconst32 ']' single_sort_clause ::= 'ORDER' 'BY' sortby @@ -3902,10 +3901,10 @@ create_as_params ::= col_def_list ::= ( col_def ) ( ( ',' col_def ) )* -type_function_name ::= - 'identifier' - | unreserved_keyword - | type_func_name_keyword +func_name ::= + type_function_name + | prefixed_column_path + | 'INDEX' opt_existing_window_name ::= name @@ -3946,6 +3945,11 @@ trim_list ::= | 'FROM' expr_list | expr_list +type_function_name ::= + 'identifier' + | unreserved_keyword + | type_func_name_keyword + char_aliases ::= 'CHAR' | 'CHARACTER' diff --git a/pkg/ccl/changefeedccl/cdceval/func_resolver.go b/pkg/ccl/changefeedccl/cdceval/func_resolver.go index 718f057dcde3..869bb60a4155 100644 --- a/pkg/ccl/changefeedccl/cdceval/func_resolver.go +++ b/pkg/ccl/changefeedccl/cdceval/func_resolver.go @@ -88,13 +88,13 @@ func (rs *cdcFunctionResolver) ResolveFunction( // ResolveFunctionByOID implements FunctionReferenceResolver interface. func (rs *cdcFunctionResolver) ResolveFunctionByOID( ctx context.Context, oid oid.Oid, -) (string, *tree.Overload, error) { +) (*tree.FunctionName, *tree.Overload, error) { fnName, overload, err := rs.wrapped.ResolveFunctionByOID(ctx, oid) if err != nil { - return "", nil, err + return nil, nil, err } - if err := checkOverloadSupported(fnName, overload); err != nil { - return "", nil, err + if err := checkOverloadSupported(fnName.Object(), overload); err != nil { + return nil, nil, err } return fnName, overload, err } diff --git a/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go b/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go index 2d5861e052e2..db724cb12ccf 100644 --- a/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go +++ b/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go @@ -2026,6 +2026,13 @@ func TestTenantLogic_udf( runLogicTest(t, "udf") } +func TestTenantLogic_udf_oid_ref( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_oid_ref") +} + func TestTenantLogic_udf_star( t *testing.T, ) { diff --git a/pkg/sql/faketreeeval/evalctx.go b/pkg/sql/faketreeeval/evalctx.go index 13fcfef5fc4f..1a886005efb2 100644 --- a/pkg/sql/faketreeeval/evalctx.go +++ b/pkg/sql/faketreeeval/evalctx.go @@ -446,8 +446,8 @@ func (ep *DummyEvalPlanner) ResolveFunction( // ResolveFunctionByOID implements FunctionReferenceResolver interface. func (ep *DummyEvalPlanner) ResolveFunctionByOID( ctx context.Context, oid oid.Oid, -) (string, *tree.Overload, error) { - return "", nil, errors.AssertionFailedf("ResolveFunctionByOID unimplemented") +) (*tree.FunctionName, *tree.Overload, error) { + return nil, nil, errors.AssertionFailedf("ResolveFunctionByOID unimplemented") } // GetMultiregionConfig is part of the eval.Planner interface. diff --git a/pkg/sql/importer/import_table_creation.go b/pkg/sql/importer/import_table_creation.go index 2fe3fe9d17a1..173a72debcb8 100644 --- a/pkg/sql/importer/import_table_creation.go +++ b/pkg/sql/importer/import_table_creation.go @@ -431,6 +431,6 @@ func (r fkResolver) ResolveFunction( // ResolveFunctionByOID implements the resolver.SchemaResolver interface. func (r fkResolver) ResolveFunctionByOID( ctx context.Context, oid oid.Oid, -) (string, *tree.Overload, error) { - return "", nil, errSchemaResolver +) (*tree.FunctionName, *tree.Overload, error) { + return nil, nil, errSchemaResolver } diff --git a/pkg/sql/logictest/testdata/logic_test/udf_oid_ref b/pkg/sql/logictest/testdata/logic_test/udf_oid_ref new file mode 100644 index 000000000000..09b15c853737 --- /dev/null +++ b/pkg/sql/logictest/testdata/logic_test/udf_oid_ref @@ -0,0 +1,134 @@ +# 1074 is the OID of builtin function "string_to_array" with signature +# "string_to_array(str: string, delimiter: string) -> string[]". +query T +SELECT [FUNCTION 1074]('hello,world', ',') +---- +{hello,world} + +statement ok +CREATE TABLE t1(a INT PRIMARY KEY, b STRING DEFAULT ([FUNCTION 1074]('hello,world', ','))) + +statement ok +INSERT INTO t1(a) VALUES (1) + +query IT +SELECT * FROM t1 +---- +1 {hello,world} + +statement ok +INSERT INTO t1 VALUES (2, 'hello,new,world') + +statement ok +CREATE INDEX idx ON t1([FUNCTION 1074](b,',')) + +query IT +SELECT * FROM t1@idx WHERE [FUNCTION 1074](b, ',') = ARRAY['hello','new','world'] +---- +2 hello,new,world + +# 814 is the OID of builtin function "length" with signature +# "length(val: string) -> int". +statement ok +ALTER TABLE t1 ADD CONSTRAINT c_len CHECK ([FUNCTION 814](b) > 2) + +statement error pq: failed to satisfy CHECK constraint \(length\(b\) > 2:::INT8\) +INSERT INTO t1 VALUES (3, 'a') + +statement ok +CREATE FUNCTION f1() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$ + +let $fn_oid +SELECT oid FROM pg_catalog.pg_proc WHERE proname = 'f1' + +query I +SELECT [FUNCTION $fn_oid]() +---- +1 + +statement ok +CREATE FUNCTION f2(a STRING) RETURNS STRING LANGUAGE SQL AS $$ SELECT a $$ + +let $fn_oid +SELECT oid FROM pg_catalog.pg_proc WHERE proname = 'f2' + +query T +SELECT [FUNCTION $fn_oid]('hello world') +---- +hello world + +# Make sure that argument types are still checked even we know which function +# overload to use. +statement error pq: unknown signature: f2\(int\) +SELECT [FUNCTION $fn_oid](123) + +# Make sure that renaming does not break the reference. +statement ok +ALTER FUNCTION f2(STRING) RENAME TO f2_new; + +query T +SELECT [FUNCTION $fn_oid]('hello world') +---- +hello world + +statement ok +CREATE SCHEMA sc1; + +statement ok +ALTER FUNCTION f2_new(STRING) SET SCHEMA sc1; + +query T +SELECT [FUNCTION $fn_oid]('hello world') +---- +hello world + +# Make sure that function dropped cannot be resolved. +statement ok +DROP FUNCTION sc1.f2_new(STRING); + +statement error function undefined +SELECT [FUNCTION $fn_oid]('maybe') + +# Referencing UDF OID within a UDF + +statement ok +CREATE FUNCTION f_in_udf() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$; + +let $fn_oid +SELECT oid FROM pg_catalog.pg_proc WHERE proname = 'f_in_udf' + +# TODO(chengxiong,mgartner): Fix this test when we enable support of calling UDFs from UDFs. +statement error pq: function \d+ not found: function undefined +CREATE FUNCTION f_using_udf() RETURNS INT LANGUAGE SQL AS $$ SELECT [FUNCTION $fn_oid]() $$; + +# 814 is the OID of builtin function "length" with signature, and it's ok to +# call it from a UDF. +statement ok +CREATE FUNCTION f_using_udf() RETURNS INT LANGUAGE SQL AS $$ SELECT [FUNCTION 814]('abc') $$; + +query I +SELECT f_using_udf(); +---- +3 + +# Make sure cross-db reference by OID is ok + +statement ok +CREATE DATABASE db1 + +statement ok +USE db1 + +statement ok +CREATE FUNCTION f_cross_db() RETURNS INT LANGUAGE SQL AS $$ SELECT 321 $$; + +let $fn_oid +SELECT oid FROM pg_catalog.pg_proc WHERE proname = 'f_cross_db' + +statement ok +USE test + +query I +SELECT [FUNCTION $fn_oid]() +---- +321 diff --git a/pkg/sql/logictest/tests/fakedist-disk/generated_test.go b/pkg/sql/logictest/tests/fakedist-disk/generated_test.go index db57579ea72d..99d65a63c9cd 100644 --- a/pkg/sql/logictest/tests/fakedist-disk/generated_test.go +++ b/pkg/sql/logictest/tests/fakedist-disk/generated_test.go @@ -1997,6 +1997,13 @@ func TestLogic_udf( runLogicTest(t, "udf") } +func TestLogic_udf_oid_ref( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_oid_ref") +} + func TestLogic_udf_star( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go b/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go index af7dda52344c..f67c56785424 100644 --- a/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go +++ b/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go @@ -2004,6 +2004,13 @@ func TestLogic_udf( runLogicTest(t, "udf") } +func TestLogic_udf_oid_ref( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_oid_ref") +} + func TestLogic_udf_star( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/fakedist/generated_test.go b/pkg/sql/logictest/tests/fakedist/generated_test.go index 04b778bec3fd..7711fc358b5e 100644 --- a/pkg/sql/logictest/tests/fakedist/generated_test.go +++ b/pkg/sql/logictest/tests/fakedist/generated_test.go @@ -2018,6 +2018,13 @@ func TestLogic_udf( runLogicTest(t, "udf") } +func TestLogic_udf_oid_ref( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_oid_ref") +} + func TestLogic_udf_star( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/local-legacy-schema-changer/generated_test.go b/pkg/sql/logictest/tests/local-legacy-schema-changer/generated_test.go index 7eeff9b87b25..79151027bb85 100644 --- a/pkg/sql/logictest/tests/local-legacy-schema-changer/generated_test.go +++ b/pkg/sql/logictest/tests/local-legacy-schema-changer/generated_test.go @@ -1983,6 +1983,13 @@ func TestLogic_udf( runLogicTest(t, "udf") } +func TestLogic_udf_oid_ref( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_oid_ref") +} + func TestLogic_udf_star( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/local-vec-off/generated_test.go b/pkg/sql/logictest/tests/local-vec-off/generated_test.go index 01242b3103b3..d1b58cf26422 100644 --- a/pkg/sql/logictest/tests/local-vec-off/generated_test.go +++ b/pkg/sql/logictest/tests/local-vec-off/generated_test.go @@ -2018,6 +2018,13 @@ func TestLogic_udf( runLogicTest(t, "udf") } +func TestLogic_udf_oid_ref( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_oid_ref") +} + func TestLogic_udf_star( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/local/generated_test.go b/pkg/sql/logictest/tests/local/generated_test.go index a0ca85621557..f20d9eaecf98 100644 --- a/pkg/sql/logictest/tests/local/generated_test.go +++ b/pkg/sql/logictest/tests/local/generated_test.go @@ -2207,6 +2207,13 @@ func TestLogic_udf( runLogicTest(t, "udf") } +func TestLogic_udf_oid_ref( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_oid_ref") +} + func TestLogic_udf_star( t *testing.T, ) { diff --git a/pkg/sql/opt/cat/catalog.go b/pkg/sql/opt/cat/catalog.go index 8764dba68bc8..fac86778d84b 100644 --- a/pkg/sql/opt/cat/catalog.go +++ b/pkg/sql/opt/cat/catalog.go @@ -155,7 +155,7 @@ type Catalog interface { ) (*tree.ResolvedFunctionDefinition, error) // ResolveFunctionByOID resolves a function overload by OID. - ResolveFunctionByOID(ctx context.Context, oid oid.Oid) (string, *tree.Overload, error) + ResolveFunctionByOID(ctx context.Context, oid oid.Oid) (*tree.FunctionName, *tree.Overload, error) // CheckPrivilege verifies that the current user has the given privilege on // the given catalog object. If not, then CheckPrivilege returns an error. diff --git a/pkg/sql/opt/testutils/testcat/function.go b/pkg/sql/opt/testutils/testcat/function.go index 7adf8591c870..1c397e11dc5b 100644 --- a/pkg/sql/opt/testutils/testcat/function.go +++ b/pkg/sql/opt/testutils/testcat/function.go @@ -51,8 +51,8 @@ func (tc *Catalog) ResolveFunction( // ResolveFunctionByOID part of the tree.FunctionReferenceResolver interface. func (tc *Catalog) ResolveFunctionByOID( ctx context.Context, oid oid.Oid, -) (string, *tree.Overload, error) { - return "", nil, errors.AssertionFailedf("ResolveFunctionByOID not supported in test catalog") +) (*tree.FunctionName, *tree.Overload, error) { + return nil, nil, errors.AssertionFailedf("ResolveFunctionByOID not supported in test catalog") } // CreateFunction handles the CREATE FUNCTION statement. diff --git a/pkg/sql/opt_catalog.go b/pkg/sql/opt_catalog.go index 1de47da98aeb..44f0a50e40bc 100644 --- a/pkg/sql/opt_catalog.go +++ b/pkg/sql/opt_catalog.go @@ -344,7 +344,7 @@ func (oc *optCatalog) ResolveFunction( func (oc *optCatalog) ResolveFunctionByOID( ctx context.Context, oid oid.Oid, -) (string, *tree.Overload, error) { +) (*tree.FunctionName, *tree.Overload, error) { return oc.planner.ResolveFunctionByOID(ctx, oid) } diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index eb3a743c4f76..bdf279e8c7a5 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -734,6 +734,9 @@ func (u *sqlSymUnion) scrubOption() tree.ScrubOption { func (u *sqlSymUnion) resolvableFuncRefFromName() tree.ResolvableFunctionReference { return tree.ResolvableFunctionReference{FunctionReference: u.unresolvedName()} } +func (u *sqlSymUnion) resolvableFuncRef() tree.ResolvableFunctionReference { + return u.val.(tree.ResolvableFunctionReference) +} func (u *sqlSymUnion) rowsFromExpr() *tree.RowsFromExpr { return u.val.(*tree.RowsFromExpr) } @@ -1370,6 +1373,7 @@ func (u *sqlSymUnion) showTenantOpts() tree.ShowTenantOptions { %type role_option password_clause valid_until_clause %type subquery_op %type <*tree.UnresolvedName> func_name func_name_no_crdb_extra +%type func_application_name %type opt_class opt_collate %type cursor_name database_name index_name opt_index_name column_name insert_column_item statistics_name window_name opt_in_database @@ -14267,7 +14271,7 @@ d_expr: if err != nil { return setErr(sqllex, err) } $$.val = d } -| func_name '(' expr_list opt_sort_clause ')' SCONST { return unimplemented(sqllex, $1.unresolvedName().String() + "(...) SCONST") } +| func_application_name '(' expr_list opt_sort_clause ')' SCONST { return unimplemented(sqllex, $1.resolvableFuncRef().String() + "(...) SCONST") } | typed_literal { $$.val = $1.expr() @@ -14354,31 +14358,44 @@ d_expr: | GROUPING '(' expr_list ')' { return unimplemented(sqllex, "d_expr grouping") } func_application: - func_name '(' ')' + func_application_name '(' ')' { - $$.val = &tree.FuncExpr{Func: $1.resolvableFuncRefFromName()} + $$.val = &tree.FuncExpr{Func: $1.resolvableFuncRef()} } -| func_name '(' expr_list opt_sort_clause ')' +| func_application_name '(' expr_list opt_sort_clause ')' { - $$.val = &tree.FuncExpr{Func: $1.resolvableFuncRefFromName(), Exprs: $3.exprs(), OrderBy: $4.orderBy(), AggType: tree.GeneralAgg} + $$.val = &tree.FuncExpr{Func: $1.resolvableFuncRef(), Exprs: $3.exprs(), OrderBy: $4.orderBy(), AggType: tree.GeneralAgg} } -| func_name '(' VARIADIC a_expr opt_sort_clause ')' { return unimplemented(sqllex, "variadic") } -| func_name '(' expr_list ',' VARIADIC a_expr opt_sort_clause ')' { return unimplemented(sqllex, "variadic") } -| func_name '(' ALL expr_list opt_sort_clause ')' +| func_application_name '(' VARIADIC a_expr opt_sort_clause ')' { return unimplemented(sqllex, "variadic") } +| func_application_name '(' expr_list ',' VARIADIC a_expr opt_sort_clause ')' { return unimplemented(sqllex, "variadic") } +| func_application_name '(' ALL expr_list opt_sort_clause ')' { - $$.val = &tree.FuncExpr{Func: $1.resolvableFuncRefFromName(), Type: tree.AllFuncType, Exprs: $4.exprs(), OrderBy: $5.orderBy(), AggType: tree.GeneralAgg} + $$.val = &tree.FuncExpr{Func: $1.resolvableFuncRef(), Type: tree.AllFuncType, Exprs: $4.exprs(), OrderBy: $5.orderBy(), AggType: tree.GeneralAgg} } // TODO(ridwanmsharif): Once DISTINCT is supported by window aggregates, // allow ordering to be specified below. -| func_name '(' DISTINCT expr_list ')' +| func_application_name '(' DISTINCT expr_list ')' + { + $$.val = &tree.FuncExpr{Func: $1.resolvableFuncRef(), Type: tree.DistinctFuncType, Exprs: $4.exprs()} + } +| func_application_name '(' '*' ')' { - $$.val = &tree.FuncExpr{Func: $1.resolvableFuncRefFromName(), Type: tree.DistinctFuncType, Exprs: $4.exprs()} + $$.val = &tree.FuncExpr{Func: $1.resolvableFuncRef(), Exprs: tree.Exprs{tree.StarExpr()}} } -| func_name '(' '*' ')' +| func_application_name '(' error { return helpWithFunction(sqllex, $1.resolvableFuncRef()) } + +func_application_name: + func_name { - $$.val = &tree.FuncExpr{Func: $1.resolvableFuncRefFromName(), Exprs: tree.Exprs{tree.StarExpr()}} + $$.val = $1.resolvableFuncRefFromName() + } +| '[' FUNCTION iconst32 ']' + { + id := $3.int32() + $$.val = tree.ResolvableFunctionReference{ + FunctionReference: &tree.FunctionOID{OID: oid.Oid(id)}, + } } -| func_name '(' error { return helpWithFunction(sqllex, $1.resolvableFuncRefFromName()) } // typed_literal represents expressions like INT '4', or generally SCONST. // This rule handles both the case of qualified and non-qualified typenames. diff --git a/pkg/sql/parser/testdata/select_numeric_func_expr b/pkg/sql/parser/testdata/select_numeric_func_expr new file mode 100644 index 000000000000..cc857f092192 --- /dev/null +++ b/pkg/sql/parser/testdata/select_numeric_func_expr @@ -0,0 +1,55 @@ +parse +SELECT [FUNCTION 1074]() +---- +SELECT [FUNCTION 1074]() +SELECT ([FUNCTION 1074]()) -- fully parenthesized +SELECT [FUNCTION 1074]() -- literals removed +SELECT [FUNCTION 1074]() -- identifiers removed + +parse +SELECT [FUNCTION 1074]('hello,world', ',') +---- +SELECT [FUNCTION 1074]('hello,world', ',') +SELECT ([FUNCTION 1074](('hello,world'), (','))) -- fully parenthesized +SELECT [FUNCTION 1074]('_', '_') -- literals removed +SELECT [FUNCTION 1074]('hello,world', ',') -- identifiers removed + +parse +SELECT [FUNCTION 1074]([FUNCTION 1074]('hello,world', ',')) +---- +SELECT [FUNCTION 1074]([FUNCTION 1074]('hello,world', ',')) +SELECT ([FUNCTION 1074](([FUNCTION 1074](('hello,world'), (','))))) -- fully parenthesized +SELECT [FUNCTION 1074]([FUNCTION 1074]('_', '_')) -- literals removed +SELECT [FUNCTION 1074]([FUNCTION 1074]('hello,world', ',')) -- identifiers removed + +parse +SELECT [FUNCTION 1074](*) +---- +SELECT [FUNCTION 1074](*) +SELECT ([FUNCTION 1074]((*))) -- fully parenthesized +SELECT [FUNCTION 1074](*) -- literals removed +SELECT [FUNCTION 1074](*) -- identifiers removed + +parse +SELECT [FUNCTION 1074]('hello','word' ORDER BY a) +---- +SELECT [FUNCTION 1074]('hello', 'word' ORDER BY a) -- normalized! +SELECT ([FUNCTION 1074](('hello'), ('word') ORDER BY (a))) -- fully parenthesized +SELECT [FUNCTION 1074]('_', '_' ORDER BY a) -- literals removed +SELECT [FUNCTION 1074]('hello', 'word' ORDER BY _) -- identifiers removed + +parse +SELECT [FUNCTION 1074](ALL 'hello', 'world' ORDER BY a) +---- +SELECT [FUNCTION 1074](ALL 'hello', 'world' ORDER BY a) +SELECT ([FUNCTION 1074](ALL ('hello'), ('world') ORDER BY (a))) -- fully parenthesized +SELECT [FUNCTION 1074](ALL '_', '_' ORDER BY a) -- literals removed +SELECT [FUNCTION 1074](ALL 'hello', 'world' ORDER BY _) -- identifiers removed + +parse +SELECT [FUNCTION 1074](DISTINCT 'hello', 'world') +---- +SELECT [FUNCTION 1074](DISTINCT 'hello', 'world') +SELECT ([FUNCTION 1074](DISTINCT ('hello'), ('world'))) -- fully parenthesized +SELECT [FUNCTION 1074](DISTINCT '_', '_') -- literals removed +SELECT [FUNCTION 1074](DISTINCT 'hello', 'world') -- identifiers removed diff --git a/pkg/sql/pg_catalog.go b/pkg/sql/pg_catalog.go index 367f8fce8b15..fce6a7be4c52 100644 --- a/pkg/sql/pg_catalog.go +++ b/pkg/sql/pg_catalog.go @@ -2642,7 +2642,7 @@ https://www.postgresql.org/docs/9.5/catalog-pg-proc.html`, return false, err } - err = addPgProcBuiltinRow(name, addRow) + err = addPgProcBuiltinRow(name.Object(), addRow) if err != nil { return false, err } diff --git a/pkg/sql/schema_resolver.go b/pkg/sql/schema_resolver.go index 75c72c074967..d8170f810462 100644 --- a/pkg/sql/schema_resolver.go +++ b/pkg/sql/schema_resolver.go @@ -97,7 +97,10 @@ func (sr *schemaResolver) GetObjectNamesAndIDs( func (sr *schemaResolver) MustGetCurrentSessionDatabase( ctx context.Context, ) (catalog.DatabaseDescriptor, error) { - return sr.descCollection.ByName(sr.txn).Get().Database(ctx, sr.CurrentDatabase()) + if sr.skipDescriptorCache { + return sr.descCollection.ByName(sr.txn).Get().Database(ctx, sr.CurrentDatabase()) + } + return sr.descCollection.ByNameWithLeased(sr.txn).Get().Database(ctx, sr.CurrentDatabase()) } // CurrentSearchPath implements the resolver.SchemaResolver interface. @@ -534,31 +537,31 @@ func maybeLookUpUDF( func (sr *schemaResolver) ResolveFunctionByOID( ctx context.Context, oid oid.Oid, -) (name string, fn *tree.Overload, err error) { +) (name *tree.FunctionName, fn *tree.Overload, err error) { if !funcdesc.IsOIDUserDefinedFunc(oid) { - name, ok := tree.OidToBuiltinName[oid] + qol, ok := tree.OidToQualifiedBuiltinOverload[oid] if !ok { - return "", nil, errors.Wrapf(tree.ErrFunctionUndefined, "function %d not found", oid) - } - funcDef := tree.FunDefs[name] - for _, o := range funcDef.Definition { - if o.Oid == oid { - return funcDef.Name, o, nil - } + return nil, nil, errors.Wrapf(tree.ErrFunctionUndefined, "function %d not found", oid) } + fnName := tree.MakeQualifiedFunctionName(sr.CurrentDatabase(), qol.Schema, tree.OidToBuiltinName[oid]) + return &fnName, qol.Overload, nil } g := sr.byIDGetterBuilder().WithoutNonPublic().WithoutOtherParent(sr.typeResolutionDbID).Get() descID := funcdesc.UserDefinedFunctionOIDToID(oid) funcDesc, err := g.Function(ctx, descID) if err != nil { - return "", nil, err + return nil, nil, err } ret, err := funcDesc.ToOverload() if err != nil { - return "", nil, err + return nil, nil, err + } + fnName, err := sr.getQualifiedFunctionName(ctx, funcDesc) + if err != nil { + return nil, nil, err } - return funcDesc.GetName(), ret, nil + return fnName, ret, nil } // NewSkippingCacheSchemaResolver constructs a schemaResolver which always skip diff --git a/pkg/sql/schemachanger/scdeps/build_deps.go b/pkg/sql/schemachanger/scdeps/build_deps.go index 23db6af338fd..a03822fb48d7 100644 --- a/pkg/sql/schemachanger/scdeps/build_deps.go +++ b/pkg/sql/schemachanger/scdeps/build_deps.go @@ -238,7 +238,7 @@ func (d *buildDeps) ResolveFunction( // ResolveFunctionByOID implements the scbuild.CatalogReader interface. func (d *buildDeps) ResolveFunctionByOID( ctx context.Context, oid oid.Oid, -) (string, *tree.Overload, error) { +) (*tree.FunctionName, *tree.Overload, error) { return d.schemaResolver.ResolveFunctionByOID(ctx, oid) } diff --git a/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go b/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go index 157665a630c0..3a0648eb24a2 100644 --- a/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go +++ b/pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go @@ -1218,35 +1218,39 @@ func (s *TestState) ResolveFunction( // ResolveFunctionByOID implements the scbuild.CatalogReader interface. func (s *TestState) ResolveFunctionByOID( ctx context.Context, oid oid.Oid, -) (string, *tree.Overload, error) { +) (*tree.FunctionName, *tree.Overload, error) { if !funcdesc.IsOIDUserDefinedFunc(oid) { - name, ok := tree.OidToBuiltinName[oid] + qol, ok := tree.OidToQualifiedBuiltinOverload[oid] if !ok { - return "", nil, errors.Newf("function %d not found", oid) + return nil, nil, errors.Newf("function %d not found", oid) } - funcDef := tree.FunDefs[name] - for _, o := range funcDef.Definition { - if o.Oid == oid { - return funcDef.Name, o, nil - } - } - return "", nil, errors.Newf("function %d not found", oid) + name := tree.MakeQualifiedFunctionName(s.CurrentDatabase(), qol.Schema, tree.OidToBuiltinName[oid]) + return &name, qol.Overload, nil } fnID := funcdesc.UserDefinedFunctionOIDToID(oid) desc, err := s.mustReadImmutableDescriptor(fnID) if err != nil { - return "", nil, err + return nil, nil, err } fnDesc, err := catalog.AsFunctionDescriptor(desc) if err != nil { - return "", nil, err + return nil, nil, err + } + dbDesc, err := s.mustReadImmutableDescriptor(fnDesc.GetParentID()) + if err != nil { + return nil, nil, err + } + scDesc, err := s.mustReadImmutableDescriptor(fnDesc.GetParentSchemaID()) + if err != nil { + return nil, nil, err } ol, err := fnDesc.ToOverload() if err != nil { - return "", nil, err + return nil, nil, err } - return fnDesc.GetName(), ol, nil + name := tree.MakeQualifiedFunctionName(dbDesc.GetName(), scDesc.GetName(), fnDesc.GetName()) + return &name, ol, nil } // ZoneConfigGetter implements scexec.Dependencies. diff --git a/pkg/sql/sem/builtins/BUILD.bazel b/pkg/sql/sem/builtins/BUILD.bazel index 6b5c5c9a3f17..ef4f0c161a07 100644 --- a/pkg/sql/sem/builtins/BUILD.bazel +++ b/pkg/sql/sem/builtins/BUILD.bazel @@ -206,6 +206,7 @@ go_test( "//pkg/util/syncutil", "//pkg/util/timeutil", "@com_github_lib_pq//:pq", + "@com_github_lib_pq//oid", "@com_github_stretchr_testify//assert", "@com_github_stretchr_testify//require", ], diff --git a/pkg/sql/sem/builtins/all_builtins.go b/pkg/sql/sem/builtins/all_builtins.go index a76ced6a02b6..b32ecff1adb3 100644 --- a/pkg/sql/sem/builtins/all_builtins.go +++ b/pkg/sql/sem/builtins/all_builtins.go @@ -20,6 +20,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/errors" + "github.com/lib/pq/oid" ) var allBuiltinNames stringSet @@ -50,6 +51,7 @@ func AllWindowBuiltinNames() []string { func init() { tree.FunDefs = make(map[string]*tree.FunctionDefinition) tree.ResolvedBuiltinFuncDefs = make(map[string]*tree.ResolvedFunctionDefinition) + tree.OidToQualifiedBuiltinOverload = make(map[oid.Oid]tree.QualifiedOverload) builtinsregistry.AddSubscription(func(name string, props *tree.FunctionProperties, overloads []tree.Overload) { for i, fn := range overloads { @@ -57,7 +59,7 @@ func init() { overloads[i].Oid = signatureMustHaveHardcodedOID(signature) } fDef := tree.NewFunctionDefinition(name, props, overloads) - addResolvedFuncDef(tree.ResolvedBuiltinFuncDefs, fDef) + addResolvedFuncDef(tree.ResolvedBuiltinFuncDefs, tree.OidToQualifiedBuiltinOverload, fDef) tree.FunDefs[name] = fDef if !fDef.ShouldDocument() { // Avoid listing help for undocumented functions. @@ -75,7 +77,9 @@ func init() { } func addResolvedFuncDef( - resolved map[string]*tree.ResolvedFunctionDefinition, def *tree.FunctionDefinition, + resolved map[string]*tree.ResolvedFunctionDefinition, + oidToOl map[oid.Oid]tree.QualifiedOverload, + def *tree.FunctionDefinition, ) { parts := strings.Split(def.Name, ".") if len(parts) > 2 || len(parts) == 0 { @@ -83,16 +87,22 @@ func addResolvedFuncDef( panic(errors.AssertionFailedf("invalid builtin function name: %s", def.Name)) } + var fd *tree.ResolvedFunctionDefinition if len(parts) == 2 { - resolved[def.Name] = tree.QualifyBuiltinFunctionDefinition(def, parts[0]) + fd = tree.QualifyBuiltinFunctionDefinition(def, parts[0]) + resolved[def.Name] = fd return + } else { + resolvedName := catconstants.PgCatalogName + "." + def.Name + fd = tree.QualifyBuiltinFunctionDefinition(def, catconstants.PgCatalogName) + resolved[resolvedName] = fd + if def.AvailableOnPublicSchema { + resolvedName = catconstants.PublicSchemaName + "." + def.Name + resolved[resolvedName] = tree.QualifyBuiltinFunctionDefinition(def, catconstants.PublicSchemaName) + } } - - resolvedName := catconstants.PgCatalogName + "." + def.Name - resolved[resolvedName] = tree.QualifyBuiltinFunctionDefinition(def, catconstants.PgCatalogName) - if def.AvailableOnPublicSchema { - resolvedName = catconstants.PublicSchemaName + "." + def.Name - resolved[resolvedName] = tree.QualifyBuiltinFunctionDefinition(def, catconstants.PublicSchemaName) + for _, o := range fd.Overloads { + oidToOl[o.Oid] = o } } diff --git a/pkg/sql/sem/builtins/all_builtins_test.go b/pkg/sql/sem/builtins/all_builtins_test.go index c1a1efc3d532..b9dd72c4a494 100644 --- a/pkg/sql/sem/builtins/all_builtins_test.go +++ b/pkg/sql/sem/builtins/all_builtins_test.go @@ -24,6 +24,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/testutils/datapathutils" "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/lib/pq/oid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -171,45 +172,62 @@ func TestAddResolvedFuncDef(t *testing.T) { defer leaktest.AfterTest(t)() testCases := []struct { - def *tree.FunctionDefinition - resolved map[string]*tree.ResolvedFunctionDefinition + def *tree.FunctionDefinition + resolved map[string]*tree.ResolvedFunctionDefinition + oidToOverload map[oid.Oid]tree.QualifiedOverload }{ { - def: &tree.FunctionDefinition{Name: "crdb_internal.fun", Definition: []*tree.Overload{{}, {}}}, + def: &tree.FunctionDefinition{Name: "crdb_internal.fun", Definition: []*tree.Overload{{Oid: 1}, {Oid: 2}}}, resolved: map[string]*tree.ResolvedFunctionDefinition{ "crdb_internal.fun": { Name: "crdb_internal.fun", Overloads: []tree.QualifiedOverload{ { Schema: "crdb_internal", - Overload: &tree.Overload{}, + Overload: &tree.Overload{Oid: 1}, }, { Schema: "crdb_internal", - Overload: &tree.Overload{}, + Overload: &tree.Overload{Oid: 2}, }, }, }, }, + oidToOverload: map[oid.Oid]tree.QualifiedOverload{ + 1: { + Schema: "crdb_internal", + Overload: &tree.Overload{Oid: 1}, + }, + 2: { + Schema: "crdb_internal", + Overload: &tree.Overload{Oid: 2}, + }, + }, }, { - def: &tree.FunctionDefinition{Name: "fun", Definition: []*tree.Overload{{}}}, + def: &tree.FunctionDefinition{Name: "fun", Definition: []*tree.Overload{{Oid: 1}}}, resolved: map[string]*tree.ResolvedFunctionDefinition{ "pg_catalog.fun": { Name: "fun", Overloads: []tree.QualifiedOverload{ { Schema: "pg_catalog", - Overload: &tree.Overload{}, + Overload: &tree.Overload{Oid: 1}, }, }, }, }, + oidToOverload: map[oid.Oid]tree.QualifiedOverload{ + 1: { + Schema: "pg_catalog", + Overload: &tree.Overload{Oid: 1}, + }, + }, }, { def: &tree.FunctionDefinition{ Name: "fun", - Definition: []*tree.Overload{{}}, + Definition: []*tree.Overload{{Oid: 1}}, FunctionProperties: tree.FunctionProperties{AvailableOnPublicSchema: true}, }, resolved: map[string]*tree.ResolvedFunctionDefinition{ @@ -218,7 +236,7 @@ func TestAddResolvedFuncDef(t *testing.T) { Overloads: []tree.QualifiedOverload{ { Schema: "pg_catalog", - Overload: &tree.Overload{}, + Overload: &tree.Overload{Oid: 1}, }, }, }, @@ -227,18 +245,25 @@ func TestAddResolvedFuncDef(t *testing.T) { Overloads: []tree.QualifiedOverload{ { Schema: "public", - Overload: &tree.Overload{}, + Overload: &tree.Overload{Oid: 1}, }, }, }, }, + oidToOverload: map[oid.Oid]tree.QualifiedOverload{ + 1: { + Schema: "pg_catalog", + Overload: &tree.Overload{Oid: 1}, + }, + }, }, } for i, tc := range testCases { t.Run(strconv.Itoa(i), func(t *testing.T) { resolved := make(map[string]*tree.ResolvedFunctionDefinition) - addResolvedFuncDef(resolved, tc.def) + oidToOverload := make(map[oid.Oid]tree.QualifiedOverload) + addResolvedFuncDef(resolved, oidToOverload, tc.def) require.Equal(t, tc.resolved, resolved) }) } diff --git a/pkg/sql/sem/eval/cast.go b/pkg/sql/sem/eval/cast.go index 344f81934746..14912a92d747 100644 --- a/pkg/sql/sem/eval/cast.go +++ b/pkg/sql/sem/eval/cast.go @@ -1000,7 +1000,7 @@ func performIntToOidCast( } return nil, err } - return tree.NewDOidWithTypeAndName(o, t, name), nil + return tree.NewDOidWithTypeAndName(o, t, name.Object()), nil default: if v == 0 { diff --git a/pkg/sql/sem/tree/function_definition.go b/pkg/sql/sem/tree/function_definition.go index bc18a674cc87..2f5e0f7008ab 100644 --- a/pkg/sql/sem/tree/function_definition.go +++ b/pkg/sql/sem/tree/function_definition.go @@ -198,6 +198,10 @@ var ResolvedBuiltinFuncDefs map[string]*ResolvedFunctionDefinition // package because of dependency issues: we can't use oidHasher from this file. var OidToBuiltinName map[oid.Oid]string +// OidToQualifiedBuiltinOverload is a map from builtin function OID to an +// qualified overload. +var OidToQualifiedBuiltinOverload map[oid.Oid]QualifiedOverload + // Format implements the NodeFormatter interface. func (fd *FunctionDefinition) Format(ctx *FmtCtx) { ctx.WriteString(fd.Name) @@ -371,6 +375,19 @@ func GetBuiltinFuncDefinitionOrFail( return def, nil } +// GetBuiltinFunctionByOIDOrFail retrieves a builtin function by OID. +func GetBuiltinFunctionByOIDOrFail(oid oid.Oid) (*ResolvedFunctionDefinition, error) { + ol, ok := OidToQualifiedBuiltinOverload[oid] + if !ok { + return nil, errors.Wrapf(ErrFunctionUndefined, "function %d not found", oid) + } + fd := &ResolvedFunctionDefinition{ + Name: OidToBuiltinName[oid], + Overloads: []QualifiedOverload{ol}, + } + return fd, nil +} + // GetBuiltinFuncDefinition search for a builtin function given a function name // and a search path. If function name is prefixed, only the builtin functions // in the specific schema are searched. Otherwise, all schemas on the given diff --git a/pkg/sql/sem/tree/function_name.go b/pkg/sql/sem/tree/function_name.go index d95ef511d78b..4fe8f99519d4 100644 --- a/pkg/sql/sem/tree/function_name.go +++ b/pkg/sql/sem/tree/function_name.go @@ -57,7 +57,7 @@ type FunctionReferenceResolver interface { // there is no function with the same oid. ResolveFunctionByOID( ctx context.Context, oid oid.Oid, - ) (string, *Overload, error) + ) (*FunctionName, *Overload, error) } // ResolvableFunctionReference implements the editable reference call of a @@ -113,6 +113,20 @@ func (ref *ResolvableFunctionReference) Resolve( } ref.FunctionReference = fd return fd, nil + case *FunctionOID: + if resolver == nil { + return GetBuiltinFunctionByOIDOrFail(t.OID) + } + fnName, o, err := resolver.ResolveFunctionByOID(ctx, t.OID) + if err != nil { + return nil, err + } + fd := &ResolvedFunctionDefinition{ + Name: fnName.Object(), + Overloads: []QualifiedOverload{{Schema: fnName.Schema(), Overload: o}}, + } + ref.FunctionReference = fd + return fd, nil default: return nil, errors.AssertionFailedf("unknown resolvable function reference type %s", t) } @@ -146,3 +160,16 @@ var _ FunctionReference = &ResolvedFunctionDefinition{} func (*UnresolvedName) functionReference() {} func (*FunctionDefinition) functionReference() {} func (*ResolvedFunctionDefinition) functionReference() {} +func (*FunctionOID) functionReference() {} + +type FunctionOID struct { + OID oid.Oid +} + +func (o *FunctionOID) String() string { + return AsString(o) +} + +func (o *FunctionOID) Format(ctx *FmtCtx) { + ctx.WriteString(fmt.Sprintf("[FUNCTION %d]", o.OID)) +} diff --git a/pkg/sql/sem/tree/type_check.go b/pkg/sql/sem/tree/type_check.go index 88b41b965ee7..c8ef0c79ca59 100644 --- a/pkg/sql/sem/tree/type_check.go +++ b/pkg/sql/sem/tree/type_check.go @@ -1040,6 +1040,9 @@ func (expr *FuncExpr) TypeCheck( searchPath = semaCtx.SearchPath resolver = semaCtx.FunctionResolver } + + _, functionResolvedByID := expr.Func.FunctionReference.(*FunctionOID) + def, err := expr.Func.Resolve(ctx, searchPath, resolver) if err != nil { return nil, err @@ -1144,13 +1147,21 @@ func (expr *FuncExpr) TypeCheck( return nil, pgerror.Newf(pgcode.UndefinedFunction, "unknown signature: %s", getFuncSig(expr, s.typedExprs, desired)) } - // Get overloads from the most significant schema in search path. - favoredOverload, err := getMostSignificantOverload( - def.Overloads, s.overloads, s.overloadIdxs, searchPath, expr, - func() string { return getFuncSig(expr, s.typedExprs, desired) }, - ) - if err != nil { - return nil, err + var favoredOverload QualifiedOverload + if functionResolvedByID { + // If the function is resolved by OID, we know that there is always only one + // overload qualified. As long as it passes the argument type checks above, + // there is no need to worry about the search path. + favoredOverload = def.Overloads[0] + } else { + // Get overloads from the most significant schema in search path. + favoredOverload, err = getMostSignificantOverload( + def.Overloads, s.overloads, s.overloadIdxs, searchPath, expr, + func() string { return getFuncSig(expr, s.typedExprs, desired) }, + ) + if err != nil { + return nil, err + } } // Just pick the first overload from the search path.