diff --git a/pkg/sql/function_resolver_test.go b/pkg/sql/function_resolver_test.go index b9b3214133b2..241d537bbe0c 100644 --- a/pkg/sql/function_resolver_test.go +++ b/pkg/sql/function_resolver_test.go @@ -176,6 +176,7 @@ CREATE FUNCTION sc1.lower() RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ SELECT 3 $$ testName string funName tree.UnresolvedName searchPath []string + expectUDF []bool expectedBody []string expectedSchema []string expectedErr string @@ -206,6 +207,7 @@ CREATE FUNCTION sc1.lower() RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ SELECT 3 $$ testName: "function with explicit schema skip first schema in path", funName: tree.UnresolvedName{NumParts: 2, Parts: tree.NameParts{"f", "sc2", "", ""}}, searchPath: []string{"sc1", "sc2"}, + expectUDF: []bool{true}, expectedBody: []string{"SELECT 2;"}, expectedSchema: []string{"sc2"}, }, @@ -213,17 +215,32 @@ CREATE FUNCTION sc1.lower() RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ SELECT 3 $$ testName: "use functions from search path", funName: tree.UnresolvedName{NumParts: 1, Parts: tree.NameParts{"f", "", "", ""}}, searchPath: []string{"sc1", "sc2"}, + expectUDF: []bool{true, true}, expectedBody: []string{"SELECT 1;", "SELECT 2;"}, expectedSchema: []string{"sc1", "sc2"}, }, + { + testName: "builtin function schema is respected", + funName: tree.UnresolvedName{NumParts: 1, Parts: tree.NameParts{"lower", "", "", ""}}, + searchPath: []string{"sc1", "sc2"}, + expectUDF: []bool{false, true}, + expectedBody: []string{"", "SELECT 3;"}, + expectedSchema: []string{"pg_catalog", "sc1"}, + }, + { + testName: "explicit builtin function schema", + funName: tree.UnresolvedName{NumParts: 2, Parts: tree.NameParts{"lower", "pg_catalog", "", ""}}, + searchPath: []string{"sc1", "sc2"}, + expectUDF: []bool{false}, + expectedBody: []string{""}, + expectedSchema: []string{"pg_catalog"}, + }, { testName: "unsupported builtin function", funName: tree.UnresolvedName{NumParts: 1, Parts: tree.NameParts{"querytree", "", "", ""}}, searchPath: []string{"sc1", "sc2"}, expectedErr: `querytree\(\): unimplemented: this function is not yet supported`, }, - // TODO(Chengxiong): add test case for builtin function names when builtin - // OIDs are changed to fixed IDs. } var sessionData sessiondatapb.SessionData @@ -246,25 +263,31 @@ CREATE FUNCTION sc1.lower() RETURNS INT IMMUTABLE LANGUAGE SQL AS $$ SELECT 3 $$ funcResolver := planner.(tree.FunctionReferenceResolver) for _, tc := range testCases { - path := sessiondata.MakeSearchPath(tc.searchPath) - funcDef, err := funcResolver.ResolveFunction(ctx, &tc.funName, &path) - if tc.expectedErr != "" { - require.Regexp(t, tc.expectedErr, err.Error()) - continue - } - require.NoError(t, err) - - require.Equal(t, len(tc.expectedBody), len(funcDef.Overloads)) - bodies := make([]string, len(funcDef.Overloads)) - schemas := make([]string, len(funcDef.Overloads)) - for i, o := range funcDef.Overloads { - _, overload, err := funcResolver.ResolveFunctionByOID(ctx, o.Oid) + t.Run(tc.testName, func(t *testing.T) { + path := sessiondata.MakeSearchPath(tc.searchPath) + funcDef, err := funcResolver.ResolveFunction(ctx, &tc.funName, &path) + if tc.expectedErr != "" { + require.Regexp(t, tc.expectedErr, err.Error()) + return + } require.NoError(t, err) - bodies[i] = overload.Body - schemas[i] = o.Schema - } - require.Equal(t, tc.expectedBody, bodies) - require.Equal(t, tc.expectedSchema, schemas) + + require.Equal(t, len(tc.expectUDF), len(funcDef.Overloads)) + require.Equal(t, len(tc.expectedBody), len(funcDef.Overloads)) + bodies := make([]string, len(funcDef.Overloads)) + schemas := make([]string, len(funcDef.Overloads)) + isUDF := make([]bool, len(funcDef.Overloads)) + for i, o := range funcDef.Overloads { + _, overload, err := funcResolver.ResolveFunctionByOID(ctx, o.Oid) + require.NoError(t, err) + bodies[i] = overload.Body + schemas[i] = o.Schema + isUDF[i] = o.IsUDF + } + require.Equal(t, tc.expectedBody, bodies) + require.Equal(t, tc.expectedSchema, schemas) + require.Equal(t, tc.expectUDF, isUDF) + }) } return nil }) @@ -280,8 +303,6 @@ func TestFuncExprTypeCheck(t *testing.T) { defer s.Stopper().Stop(ctx) tDB := sqlutils.MakeSQLRunner(sqlDB) - // TODO(Chengxiong): add test cases with builtin function when builtin - // function OIDs are changed to fixed IDs. tDB.Exec(t, ` CREATE SCHEMA sc1; CREATE SCHEMA sc2; @@ -346,6 +367,7 @@ CREATE FUNCTION sc1.lower(a STRING) RETURNS STRING IMMUTABLE LANGUAGE SQL AS $$ exprStr: "lower('HI')", searchPath: []string{"sc1", "sc2"}, expectedFuncBody: "", + expectedFuncOID: 827, desiredType: types.String, }, { @@ -404,11 +426,8 @@ CREATE FUNCTION sc1.lower(a STRING) RETURNS STRING IMMUTABLE LANGUAGE SQL AS $$ require.False(t, funcExpr.ResolvedOverload().IsUDF) } require.False(t, funcExpr.ResolvedOverload().UDFContainsOnlySignature) - if tc.expectedFuncOID > 0 { - require.Equal(t, tc.expectedFuncOID, int(funcExpr.ResolvedOverload().Oid)) - } else { - require.False(t, funcdesc.IsOIDUserDefinedFunc(funcExpr.ResolvedOverload().Oid)) - } + require.Equal(t, tc.expectedFuncOID, int(funcExpr.ResolvedOverload().Oid)) + require.Equal(t, funcExpr.ResolvedOverload().IsUDF, funcdesc.IsOIDUserDefinedFunc(funcExpr.ResolvedOverload().Oid)) require.Equal(t, tc.expectedFuncBody, funcExpr.ResolvedOverload().Body) }) } diff --git a/pkg/sql/grant_revoke.go b/pkg/sql/grant_revoke.go index 608f77dabe7c..d42949a9a7a3 100644 --- a/pkg/sql/grant_revoke.go +++ b/pkg/sql/grant_revoke.go @@ -419,7 +419,6 @@ func (n *changeDescriptorBackedPrivilegesNode) startExec(params runParams) error FuncName: d.Name, // FIXME }) } - // TODO(chengxiong): add eventlog for function privilege changes. } } diff --git a/pkg/sql/logictest/testdata/logic_test/pg_builtins b/pkg/sql/logictest/testdata/logic_test/pg_builtins index 6d42fb197e04..3702fbc10dd7 100644 --- a/pkg/sql/logictest/testdata/logic_test/pg_builtins +++ b/pkg/sql/logictest/testdata/logic_test/pg_builtins @@ -707,11 +707,21 @@ bit_in query T SELECT to_regprocedure('bit_in') ---- +NULL + +query T +SELECT to_regprocedure('bit_in(int)') +---- bit_in query T SELECT to_regprocedure('version') ---- +NULL + +query T +SELECT to_regprocedure('version()') +---- version query T diff --git a/pkg/sql/logictest/testdata/logic_test/pg_catalog b/pkg/sql/logictest/testdata/logic_test/pg_catalog index 5aa853af6fcf..bc9b8e9d319e 100644 --- a/pkg/sql/logictest/testdata/logic_test/pg_catalog +++ b/pkg/sql/logictest/testdata/logic_test/pg_catalog @@ -4697,21 +4697,6 @@ query T SELECT proname FROM pg_catalog.pg_proc WHERE oid = 0 ---- -let $cur_max_builtin_oid -SELECT cur_max_builtin_oid FROM [SELECT max(oid) as cur_max_builtin_oid FROM pg_catalog.pg_proc] - -## If this test failed (proname is the same, but oid increased), it's likely that a -## new builtin function is implemented and it's somewhere in the middle of the -## existing functions at init time. Though the changes to builtin function OID is -## generally ok, it's still better if we could move the new implement to end of the -## list at init time (see all_builtins.go) -## TODO(chengxiong): consider to have a deterministic list of builtin function oids -## so that new implementations can just be added to it. -query TT -SELECT proname, oid FROM pg_catalog.pg_proc WHERE oid = $cur_max_builtin_oid ----- -pg_get_functiondef 2035 - ## Ensure that unnest works with oid wrapper arrays query O diff --git a/pkg/sql/logictest/testdata/logic_test/pgoidtype b/pkg/sql/logictest/testdata/logic_test/pgoidtype index 9489e86d1c34..3e829cd015a2 100644 --- a/pkg/sql/logictest/testdata/logic_test/pgoidtype +++ b/pkg/sql/logictest/testdata/logic_test/pgoidtype @@ -33,8 +33,17 @@ SELECT pg_typeof('1'::OID), pg_typeof('pg_constraint'::REGCLASS), pg_typeof('pub ---- oid regclass regnamespace +statement error pq: unknown function: upper\(string\)\(\): function undefined +SELECT 'upper(string)'::REGPROC + +statement error invalid function signature: upper +SELECT 'upper'::REGPROCEDURE + +statement error pq: function upper\(int\) does not exist: function undefined +SELECT 'upper(int)'::REGPROCEDURE + query TT -SELECT pg_typeof('upper'::REGPROC), pg_typeof('upper'::REGPROCEDURE) +SELECT pg_typeof('upper'::REGPROC), pg_typeof('upper(string)'::REGPROCEDURE) ---- regproc regprocedure @@ -81,28 +90,31 @@ WHERE relname = 'pg_constraint' 0 pg_constraint 0 pg_constraint pg_constraint query OOOO -SELECT 'upper'::REGPROC, 'upper'::REGPROCEDURE, 'pg_catalog.upper'::REGPROCEDURE, 'upper'::REGPROC::OID +SELECT 'upper'::REGPROC, 'upper(string)'::REGPROCEDURE, 'pg_catalog.upper(string)'::REGPROCEDURE, 'upper'::REGPROC::OID ---- upper upper upper 829 -query error invalid function name +query error pq: invalid function signature: invalid.more.pg_catalog.upper: at or near ".": syntax error SELECT 'invalid.more.pg_catalog.upper'::REGPROCEDURE query OOO -SELECT 'upper(int)'::REGPROC, 'upper(int)'::REGPROCEDURE, 'upper(int)'::REGPROC::OID +SELECT 'upper'::REGPROC, 'upper(string)'::REGPROCEDURE, 'upper'::REGPROC::OID ---- upper upper 829 -query error unknown function: blah\(\) -SELECT 'blah(ignored, ignored)'::REGPROC, 'blah(ignored, ignored)'::REGPROCEDURE +query error pq: unknown function: blah\(ignored\)\(\): function undefined +SELECT 'blah(ignored)'::REGPROC + +query error pq: unknown function: blah\(\): function undefined +SELECT 'blah(int, int)'::REGPROCEDURE -query error unknown function: blah\(\) +query error pq: invalid name: expected separator .: blah \( ignored , ignored \) SELECT ' blah ( ignored , ignored ) '::REGPROC -query error unknown function: blah\(\) +query error pq: invalid name: expected separator .: blah \(\) SELECT 'blah ()'::REGPROC -query error unknown function: blah\(\) +query error pq: invalid name: expected separator .: blah\( \) SELECT 'blah( )'::REGPROC query error invalid name: expected separator \.: blah\(, \) @@ -111,15 +123,15 @@ SELECT 'blah(, )'::REGPROC query error more than one function named 'sqrt' SELECT 'sqrt'::REGPROC -query OOOO -SELECT 'array_in'::REGPROC, 'array_in(a,b,c)'::REGPROC, 'pg_catalog.array_in'::REGPROC, 'pg_catalog.array_in( a ,b, c )'::REGPROC +query OO +SELECT 'array_in'::REGPROC, 'pg_catalog.array_in'::REGPROC ---- -array_in array_in array_in array_in +array_in array_in -query OOOO -SELECT 'array_in'::REGPROCEDURE, 'array_in(a,b,c)'::REGPROCEDURE, 'pg_catalog.array_in'::REGPROCEDURE, 'pg_catalog.array_in( a ,b, c )'::REGPROCEDURE +query OO +SELECT 'array_in(int)'::REGPROCEDURE, 'pg_catalog.array_in(int)'::REGPROCEDURE ---- -array_in array_in array_in array_in +array_in array_in query OO SELECT 'public'::REGNAMESPACE, 'public'::REGNAMESPACE::OID @@ -156,7 +168,7 @@ query error pgcode 42883 unknown function: blah\(\) SELECT 'blah'::REGPROC query error pgcode 42883 unknown function: blah\(\) -SELECT 'blah'::REGPROCEDURE +SELECT 'blah()'::REGPROCEDURE query error pgcode 42704 namespace 'blah' does not exist SELECT 'blah'::REGNAMESPACE @@ -472,7 +484,12 @@ SELECT 'pg_catalog."radians"'::regproc radians query T -SELECT 'pg_catalog."radians"("float4")'::regproc +SELECT 'pg_catalog."radians"'::regproc +---- +radians + +query T +SELECT 'pg_catalog."radians"("float4")'::regprocedure ---- radians diff --git a/pkg/sql/logictest/testdata/logic_test/privilege_builtins b/pkg/sql/logictest/testdata/logic_test/privilege_builtins index d0bf9223aaf8..4e172106f461 100644 --- a/pkg/sql/logictest/testdata/logic_test/privilege_builtins +++ b/pkg/sql/logictest/testdata/logic_test/privilege_builtins @@ -505,14 +505,14 @@ SELECT has_function_privilege((SELECT oid FROM pg_proc LIMIT 1), 'EXECUTE') ---- true -query error pgcode 42883 unknown function: does_not_exist() +query error pq: has_function_privilege\(\): invalid function signature: does_not_exist SELECT has_function_privilege('does_not_exist', 'EXECUTE') query error pgcode 42883 unknown function: does_not_exist() SELECT has_function_privilege('does_not_exist()', 'EXECUTE') query B -SELECT has_function_privilege('version', ' EXECUTE ') +SELECT has_function_privilege('version()', ' EXECUTE ') ---- true @@ -524,10 +524,15 @@ true query B SELECT has_function_privilege('cos(float)', 'EXECUTE WITH GRANT OPTION') ---- +false + +query B +SELECT has_function_privilege('cos(float)', 'EXECUTE, EXECUTE WITH GRANT OPTION') +---- true query B -SELECT has_function_privilege('version'::Name, 'EXECUTE') +SELECT has_function_privilege('version()'::Name, 'EXECUTE') ---- true diff --git a/pkg/sql/logictest/testdata/logic_test/udf b/pkg/sql/logictest/testdata/logic_test/udf index 1e3616f2ffc3..3399c1742927 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf +++ b/pkg/sql/logictest/testdata/logic_test/udf @@ -889,6 +889,85 @@ test public 100140 test_priv_f1() root EXECUTE test public 100143 test_priv_f2(int8) root EXECUTE true test test_priv_sc1 100144 test_priv_f3() root EXECUTE true +# Make sure has_function_privilege works. +query B +SELECT has_function_privilege('test_priv_f2(INT)', 'EXECUTE') +---- +true + +query B +SELECT has_function_privilege('test_priv_f2(INT)', 'EXECUTE WITH GRANT OPTION') +---- +true + +query B +SELECT has_function_privilege('test_priv_f2(INT)', 'EXECUTE, EXECUTE WITH GRANT OPTION') +---- +true + +user testuser + +query B +SELECT has_function_privilege('test_priv_f2(INT)', 'EXECUTE') +---- +false + +query B +SELECT has_function_privilege('test_priv_f2(INT)', 'EXECUTE WITH GRANT OPTION') +---- +false + +query B +SELECT has_function_privilege('test_priv_f2(INT)', 'EXECUTE, EXECUTE WITH GRANT OPTION') +---- +false + +user root + +statement ok +GRANT EXECUTE ON FUNCTION test_priv_f1(), test_priv_f2(int), test_priv_sc1.test_priv_f3 TO testuser WITH GRANT OPTION; + +user testuser + +query B retry +SELECT has_function_privilege('test_priv_f2(INT)', 'EXECUTE') +---- +true + +query B +SELECT has_function_privilege('test_priv_f2(INT)', 'EXECUTE WITH GRANT OPTION') +---- +true + +query B +SELECT has_function_privilege('test_priv_f2(INT)', 'EXECUTE, EXECUTE WITH GRANT OPTION') +---- +true + +user root + +statement ok +REVOKE GRANT OPTION FOR EXECUTE ON FUNCTION test_priv_f1(), test_priv_f2(int), test_priv_sc1.test_priv_f3 FROM testuser; + +user testuser + +query B retry +SELECT has_function_privilege('test_priv_f2(INT)', 'EXECUTE WITH GRANT OPTION') +---- +false + +query B +SELECT has_function_privilege('test_priv_f2(INT)', 'EXECUTE') +---- +true + +query B +SELECT has_function_privilege('test_priv_f2(INT)', 'EXECUTE, EXECUTE WITH GRANT OPTION') +---- +true + +user root + statement ok SET search_path = public; diff --git a/pkg/sql/parser/help.go b/pkg/sql/parser/help.go index 47279079203d..6b9e205f2c16 100644 --- a/pkg/sql/parser/help.go +++ b/pkg/sql/parser/help.go @@ -102,8 +102,7 @@ func helpWithFunction(sqllex sqlLexer, f tree.ResolvableFunctionReference) int { if err != nil { return 1 } - // TODO(Chengxiong): Consider how to produce proper help message for - // UDFs. + props, _ := builtinsregistry.GetBuiltinProperties(d.Name) msg := HelpMessage{ Function: f.String(), diff --git a/pkg/sql/resolver.go b/pkg/sql/resolver.go index fbb0f018815e..21e65aeddce2 100644 --- a/pkg/sql/resolver.go +++ b/pkg/sql/resolver.go @@ -351,6 +351,12 @@ func (p *planner) ResolveDescriptorForPrivilegeSpecifier( } } return table, nil + } else if specifier.FunctionOID != nil { + fnID, err := funcdesc.UserDefinedFunctionOIDToID(*specifier.FunctionOID) + if err != nil { + return nil, err + } + return p.Descriptors().GetImmutableFunctionByID(ctx, p.txn, fnID, tree.ObjectLookupFlagsWithRequired()) } return nil, errors.AssertionFailedf("invalid HasPrivilegeSpecifier") } diff --git a/pkg/sql/sem/builtins/pg_builtins.go b/pkg/sql/sem/builtins/pg_builtins.go index fa5990220fbd..c075a7529d2f 100644 --- a/pkg/sql/sem/builtins/pg_builtins.go +++ b/pkg/sql/sem/builtins/pg_builtins.go @@ -1476,33 +1476,37 @@ SELECT description oid = t } - fn, err := getNameForArg(ctx, oid, "pg_proc", "proname") + // Check if the function OID actually exists. + _, _, err := ctx.Planner.ResolveFunctionByOID(ctx.Ctx(), oid.(*tree.DOid).Oid) if err != nil { + if errors.Is(err, tree.ErrFunctionUndefined) { + return eval.ObjectNotFound, nil + } return eval.HasNoPrivilege, err } - retNull := false - if fn == "" { - // Postgres returns NULL if no matching function is found - // when given an OID. - retNull = true - } + specifier := eval.HasPrivilegeSpecifier{FunctionOID: &oid.(*tree.DOid).Oid} privs, err := parsePrivilegeStr(args[1], privMap{ - // TODO(nvanbenschoten): this privilege is incorrect, but we don't - // currently have an EXECUTE privilege and we aren't even checking - // this down below, so it's fine for now. - "EXECUTE": {Kind: privilege.USAGE}, - "EXECUTE WITH GRANT OPTION": {Kind: privilege.USAGE, GrantOption: true}, + "EXECUTE": {Kind: privilege.EXECUTE}, + "EXECUTE WITH GRANT OPTION": {Kind: privilege.EXECUTE, GrantOption: true}, }) if err != nil { return eval.HasNoPrivilege, err } - if retNull { - return eval.ObjectNotFound, nil + + // For user-defined function, utilize the descriptor based way. + if catid.IsOIDUserDefined(oid.(*tree.DOid).Oid) { + return ctx.Planner.HasAnyPrivilege(ctx.Ctx(), specifier, ctx.SessionData().User(), privs) } - // All users have EXECUTE privileges for all functions. - _ = privs - return eval.HasPrivilege, nil + + // For builtin functions, all users should have `EXECUTE` privilege, but + // no one can grant on them. + for _, priv := range privs { + if !priv.GrantOption { + return eval.HasPrivilege, nil + } + } + return eval.HasNoPrivilege, nil }, ), diff --git a/pkg/sql/sem/eval/BUILD.bazel b/pkg/sql/sem/eval/BUILD.bazel index 4296bfe5a534..baf6d6676a1e 100644 --- a/pkg/sql/sem/eval/BUILD.bazel +++ b/pkg/sql/sem/eval/BUILD.bazel @@ -51,6 +51,7 @@ go_library( "//pkg/sql/catalog/catpb", "//pkg/sql/catalog/descpb", "//pkg/sql/lex", + "//pkg/sql/parser", "//pkg/sql/pgwire/pgcode", "//pkg/sql/pgwire/pgerror", "//pkg/sql/pgwire/pgnotice", diff --git a/pkg/sql/sem/eval/deps.go b/pkg/sql/sem/eval/deps.go index 430827875d2a..4ff838958197 100644 --- a/pkg/sql/sem/eval/deps.go +++ b/pkg/sql/sem/eval/deps.go @@ -161,6 +161,11 @@ type HasPrivilegeSpecifier struct { // Only one of ColumnName, ColumnAttNum is filled. ColumnName *tree.Name ColumnAttNum *uint32 + + // Function privilege + // This needs to be a user-defined function OID. Builtin function OIDs won't + // work since they're not descriptors based. + FunctionOID *oid.Oid } // TypeResolver is an interface for resolving types and type OIDs. diff --git a/pkg/sql/sem/eval/parse_doid.go b/pkg/sql/sem/eval/parse_doid.go index 87e2d7a020e7..a9b13e666fd2 100644 --- a/pkg/sql/sem/eval/parse_doid.go +++ b/pkg/sql/sem/eval/parse_doid.go @@ -14,10 +14,13 @@ import ( "regexp" "strings" + "github.com/cockroachdb/cockroach/pkg/sql/parser" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + "github.com/cockroachdb/cockroach/pkg/sql/sem/catid" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/types" + "github.com/cockroachdb/errors" "github.com/lib/pq/oid" ) @@ -46,18 +49,14 @@ func ParseDOid(ctx *Context, s string, t *types.T) (*tree.DOid, error) { } switch t.Oid() { - case oid.T_regproc, oid.T_regprocedure: - // Trim procedure type parameters, e.g. `max(int)` becomes `max`. - // Postgres only does this when the cast is ::regprocedure, but we're - // going to always do it. - // We additionally do not yet implement disambiguation based on type - // parameters: we return the match iff there is exactly one. - s = pgSignatureRegexp.ReplaceAllString(s, "$1") - - substrs, err := splitIdentifierList(s) + case oid.T_regproc: + // To be compatible with postgres, we always treat the trimmed input string + // as a function name. + substrs, err := splitIdentifierList(strings.TrimSpace(s)) if err != nil { return nil, err } + if len(substrs) > 3 { // A fully qualified function name in pg's dialect can contain // at most 3 parts: db.schema.funname. @@ -80,6 +79,53 @@ func ParseDOid(ctx *Context, s string, t *types.T) (*tree.DOid, error) { } overload := funcDef.Overloads[0] return tree.NewDOidWithTypeAndName(overload.Oid, t, funcDef.Name), nil + case oid.T_regprocedure: + // Fake a ALTER FUNCTION statement to extract the function signature. + // We're kinda being lazy here to rely on the parser to determine if the + // function signature syntax is sane from grammar perspective. We may + // match postgres' implementation of `parseNameAndArgTypes` to return + // more detailed errors like "expected a left parenthesis". + stmt, err := parser.ParseOne("ALTER FUNCTION " + strings.TrimSpace(s) + " IMMUTABLE") + if err != nil { + return nil, errors.Wrapf(err, "invalid function signature: %s", s) + } + fn := stmt.AST.(*tree.AlterFunctionOptions).Function + if fn.Args == nil { + // Always require the full function signature. + return nil, errors.Newf("invalid function signature: %s", s) + } + + un := fn.FuncName.ToUnresolvedObjectName().ToUnresolvedName() + fd, err := ctx.Planner.ResolveFunction(ctx.Ctx(), un, &ctx.SessionData().SearchPath) + if err != nil { + return nil, err + } + + if len(fd.Overloads) == 1 { + // This is a hack to be compatible with some ORMs which depends on some + // builtin function not implemented in CRDB. We just use `Any` as the arg + // type while some ORMs sends more meaningful function signatures whose + // arg type list mismatch with `Any` type. For this case we just + // short-circuit it to return the oid. For example, `array_in` is defined + // to take in a `Any` type, but some ORM sends + // `'array_in(cstring,oid,integer)'::REGPROCEDURE` for introspection. + ol := fd.Overloads[0] + if !catid.IsOIDUserDefined(ol.Oid) && + ol.Types.Length() == 1 && + ol.Types.GetAt(0).Identical(types.Any) { + return tree.NewDOidWithTypeAndName(ol.Oid, t, fd.Name), nil + } + } + + argTypes, err := fn.InputArgTypes(ctx.Ctx(), ctx.Planner) + if err != nil { + return nil, err + } + ol, err := fd.MatchOverload(argTypes, fn.FuncName.Schema(), &ctx.SessionData().SearchPath) + if err != nil { + return nil, err + } + return tree.NewDOidWithTypeAndName(ol.Oid, t, fd.Name), nil case oid.T_regtype: parsedTyp, err := ctx.Planner.GetTypeFromValidSQLSyntax(s) if err == nil { diff --git a/pkg/sql/sem/tree/function_definition.go b/pkg/sql/sem/tree/function_definition.go index c1e6066cc022..28995dd5a1da 100644 --- a/pkg/sql/sem/tree/function_definition.go +++ b/pkg/sql/sem/tree/function_definition.go @@ -416,8 +416,8 @@ func GetBuiltinFuncDefinition( } // First try that if we can get function directly with the function name. - // There is a case where the part[0] of the name is a qualified string. - // TODO(Chengxiong): figure out why that could be an input. + // There is a case where the part[0] of the name is a qualified string when + // the qualified name is double quoted as a single name like "schema.fn". if def, ok := ResolvedBuiltinFuncDefs[fName.Object()]; ok { return def, nil }