Skip to content

Commit

Permalink
sql: use unimplemented error rather than syntax error for PL/pgSQL
Browse files Browse the repository at this point in the history
This change does not affect anything user-facing, but it allows us to
parse CREATE FUNCTION statements that use `LANGUAGE plpgsql`. The
plpgsql grammar is still not supported, so this will still show an error
to the user. However, since parsing succeeds, the statement will be sent
to telemetry logs, allowing us to analyze which parts of the plpgsql
grammer that customers are trying to use.

Release note: None
  • Loading branch information
rafiss committed Feb 7, 2023
1 parent f640acb commit 6b34dfb
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 22 deletions.
1 change: 1 addition & 0 deletions pkg/sql/catalog/funcdesc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ go_library(
"//pkg/sql/sem/tree",
"//pkg/sql/sem/volatility",
"//pkg/sql/types",
"//pkg/util/errorutil/unimplemented",
"//pkg/util/hlc",
"//pkg/util/iterutil",
"//pkg/util/protoutil",
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/catalog/funcdesc/func_desc.go
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ func (desc *immutable) getCreateExprLang() tree.FunctionLanguage {
case catpb.Function_SQL:
return tree.FunctionLangSQL
}
return 0
return tree.FunctionLangUnknown
}

func (desc *immutable) getCreateExprVolatility() tree.FunctionVolatility {
Expand Down
5 changes: 4 additions & 1 deletion pkg/sql/catalog/funcdesc/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
)

// VolatilityToProto converts sql statement input volatility to protobuf
Expand Down Expand Up @@ -54,9 +55,11 @@ func FunctionLangToProto(v tree.FunctionLanguage) (catpb.Function_Language, erro
switch v {
case tree.FunctionLangSQL:
return catpb.Function_SQL, nil
case tree.FunctionLangPlPgSQL:
return -1, unimplemented.NewWithIssue(91569, "PL/pgSQL is not yet supported")
}

return -1, pgerror.Newf(pgcode.InvalidParameterValue, "Unknown function language %q", v)
return -1, pgerror.Newf(pgcode.UndefinedObject, "language %q does not exist", v)
}

// ParamClassToProto converts sql statement input argument class to protobuf
Expand Down
18 changes: 18 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/udf
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,24 @@ CREATE TABLE ab (
b INT
)

statement error pgcode 0A000 unimplemented: PL/pgSQL is not yet supported
CREATE FUNCTION populate() RETURNS integer AS $$
DECLARE
-- declarations
BEGIN
PERFORM my_function();
END;
$$ LANGUAGE plpgsql

statement error pgcode 42704 language \"made_up_language\" does not exist
CREATE FUNCTION populate() RETURNS integer AS $$
DECLARE
-- declarations
BEGIN
PERFORM my_function();
END;
$$ LANGUAGE made_up_language

statement error pq: unimplemented: user-defined functions with SETOF return types are not supported
CREATE FUNCTION f(a int) RETURNS SETOF INT LANGUAGE SQL AS 'SELECT 1'

Expand Down
1 change: 1 addition & 0 deletions pkg/sql/opt/optbuilder/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ go_library(
"//pkg/settings",
"//pkg/sql/catalog/colinfo",
"//pkg/sql/catalog/descpb",
"//pkg/sql/catalog/funcdesc",
"//pkg/sql/catalog/schemaexpr",
"//pkg/sql/catalog/seqexpr",
"//pkg/sql/catalog/tabledesc",
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/opt/optbuilder/create_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ package optbuilder

import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/funcdesc"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc"
"github.com/cockroachdb/cockroach/pkg/sql/opt"
"github.com/cockroachdb/cockroach/pkg/sql/opt/memo"
Expand Down Expand Up @@ -81,6 +82,10 @@ func (b *Builder) buildCreateFunction(cf *tree.CreateFunction, inScope *scope) (
funcBodyStr = string(opt)
case tree.FunctionLanguage:
languageFound = true
// Check the language here, before attempting to parse the function body.
if _, err := funcdesc.FunctionLangToProto(opt); err != nil {
panic(err)
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/testutils/testcat/function.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ func collectFuncOptions(
}

case tree.FunctionLanguage:
if t != tree.FunctionLangSQL {
panic(fmt.Errorf("LANGUAGE must be SQL"))
if t != tree.FunctionLangSQL && t != tree.FunctionLangPlPgSQL {
panic(fmt.Errorf("LANGUAGE must be SQL or plpgsql"))
}

default:
Expand Down
50 changes: 50 additions & 0 deletions pkg/sql/parser/testdata/create_function
Original file line number Diff line number Diff line change
Expand Up @@ -495,3 +495,53 @@ using the support form.
We appreciate your feedback.
----
----

parse
CREATE FUNCTION populate() RETURNS integer AS $$
DECLARE
-- declarations
BEGIN
PERFORM my_function();
END;
$$ LANGUAGE plpgsql
----
CREATE FUNCTION populate()
RETURNS INT8
LANGUAGE plpgsql
AS $$
DECLARE
-- declarations
BEGIN
PERFORM my_function();
END;
$$ -- normalized!
CREATE FUNCTION populate()
RETURNS INT8
LANGUAGE plpgsql
AS $$
DECLARE
-- declarations
BEGIN
PERFORM my_function();
END;
$$ -- fully parenthesized
CREATE FUNCTION populate()
RETURNS INT8
LANGUAGE plpgsql
AS $$
DECLARE
-- declarations
BEGIN
PERFORM my_function();
END;
$$ -- literals removed
CREATE FUNCTION _()
RETURNS INT8
LANGUAGE plpgsql
AS $$
DECLARE
-- declarations
BEGIN
PERFORM my_function();
END;
$$ -- identifiers removed
37 changes: 19 additions & 18 deletions pkg/sql/sem/tree/udf.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,33 +235,34 @@ func (node FunctionLeakproof) Format(ctx *FmtCtx) {

// FunctionLanguage indicates the language of the statements in the UDF function
// body.
type FunctionLanguage int
type FunctionLanguage string

const (
_ FunctionLanguage = iota
// FunctionLangSQL represent SQL language.
FunctionLangSQL
// FunctionLangUnknown represents an unknown language.
FunctionLangUnknown FunctionLanguage = "unknown"
// FunctionLangSQL represents SQL language.
FunctionLangSQL FunctionLanguage = "SQL"
// FunctionLangPlPgSQL represents the PL/pgSQL procedural language.
FunctionLangPlPgSQL FunctionLanguage = "plpgsql"
)

// Format implements the NodeFormatter interface.
func (node FunctionLanguage) Format(ctx *FmtCtx) {
ctx.WriteString("LANGUAGE ")
switch node {
case FunctionLangSQL:
ctx.WriteString("SQL")
default:
panic(pgerror.New(pgcode.InvalidParameterValue, "Unknown function option"))
}
ctx.WriteString(string(node))
}

// AsFunctionLanguage converts a string to a FunctionLanguage if applicable.
// Error is returned if string does not represent a valid UDF language.
// No error is returned if string does not represent a valid UDF language;
// unknown languages result in an error later.
func AsFunctionLanguage(lang string) (FunctionLanguage, error) {
switch strings.ToLower(lang) {
case "sql":
return FunctionLangSQL, nil
case "plpgsql":
return FunctionLangPlPgSQL, nil
}
return 0, errors.Newf("language %q does not exist", lang)
return FunctionLanguage(lang), nil
}

// FunctionBodyStr is a string containing all statements in a UDF body.
Expand Down Expand Up @@ -523,34 +524,34 @@ func MaybeFailOnUDFUsage(expr TypedExpr) error {
// function options in the given slice.
func ValidateFuncOptions(options FunctionOptions) error {
var hasLang, hasBody, hasLeakProof, hasVolatility, hasNullInputBehavior bool
err := func(opt FunctionOption) error {
conflictingErr := func(opt FunctionOption) error {
return errors.Wrapf(ErrConflictingFunctionOption, "%s", AsString(opt))
}
for _, option := range options {
switch option.(type) {
case FunctionLanguage:
if hasLang {
return err(option)
return conflictingErr(option)
}
hasLang = true
case FunctionBodyStr:
if hasBody {
return err(option)
return conflictingErr(option)
}
hasBody = true
case FunctionLeakproof:
if hasLeakProof {
return err(option)
return conflictingErr(option)
}
hasLeakProof = true
case FunctionVolatility:
if hasVolatility {
return err(option)
return conflictingErr(option)
}
hasVolatility = true
case FunctionNullInputBehavior:
if hasNullInputBehavior {
return err(option)
return conflictingErr(option)
}
hasNullInputBehavior = true
default:
Expand Down

0 comments on commit 6b34dfb

Please sign in to comment.