From 20c43ab5a5aaa06948490de9f86e353b110e7de8 Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Thu, 27 Jul 2023 16:50:45 -0700 Subject: [PATCH] sql: prevent panic when using UDF as EXECUTE argument Fixes #99008 Release note (bug fix): A bug has been fixed that caused nodes to crash when attempting to `EXECUTE` a prepared statement with an argument that referenced a user-defined function. This bug was present since user-defined functions were introduced in version 22.2. --- pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go | 7 +++++++ pkg/sql/execute.go | 3 +-- pkg/sql/logictest/testdata/logic_test/udf_prepare | 8 ++++++++ pkg/sql/logictest/tests/fakedist-disk/generated_test.go | 7 +++++++ .../logictest/tests/fakedist-vec-off/generated_test.go | 7 +++++++ pkg/sql/logictest/tests/fakedist/generated_test.go | 7 +++++++ .../tests/local-legacy-schema-changer/generated_test.go | 7 +++++++ .../tests/local-mixed-22.2-23.1/generated_test.go | 7 +++++++ pkg/sql/logictest/tests/local-vec-off/generated_test.go | 7 +++++++ pkg/sql/logictest/tests/local/generated_test.go | 7 +++++++ pkg/sql/sem/eval/expr.go | 4 ++++ 11 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 pkg/sql/logictest/testdata/logic_test/udf_prepare diff --git a/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go b/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go index 686f4ee77fd0..c9f7ca965bc2 100644 --- a/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go +++ b/pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go @@ -2208,6 +2208,13 @@ func TestTenantLogic_udf_plpgsql( runLogicTest(t, "udf_plpgsql") } +func TestTenantLogic_udf_prepare( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_prepare") +} + func TestTenantLogic_udf_privileges( t *testing.T, ) { diff --git a/pkg/sql/execute.go b/pkg/sql/execute.go index d3f3fca1acfa..30cbda423374 100644 --- a/pkg/sql/execute.go +++ b/pkg/sql/execute.go @@ -35,7 +35,6 @@ func (p *planner) fillInPlaceholders( } qArgs := make(tree.QueryArguments, len(params)) - var semaCtx tree.SemaContext for i, e := range params { idx := tree.PlaceholderIdx(i) @@ -54,7 +53,7 @@ func (p *planner) fillInPlaceholders( } } typedExpr, err := schemaexpr.SanitizeVarFreeExpr( - ctx, e, typ, "EXECUTE parameter" /* context */, &semaCtx, volatility.Volatile, true, /*allowAssignmentCast*/ + ctx, e, typ, "EXECUTE parameter" /* context */, p.SemaCtx(), volatility.Volatile, true, /*allowAssignmentCast*/ ) if err != nil { return nil, pgerror.WithCandidateCode(err, pgcode.WrongObjectType) diff --git a/pkg/sql/logictest/testdata/logic_test/udf_prepare b/pkg/sql/logictest/testdata/logic_test/udf_prepare new file mode 100644 index 000000000000..ccaa79123d75 --- /dev/null +++ b/pkg/sql/logictest/testdata/logic_test/udf_prepare @@ -0,0 +1,8 @@ +statement ok +CREATE FUNCTION f() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$ + +statement ok +PREPARE p AS SELECT $1::INT + +statement error pgcode 0A000 cannot evaluate function in this context +EXECUTE p(f()) diff --git a/pkg/sql/logictest/tests/fakedist-disk/generated_test.go b/pkg/sql/logictest/tests/fakedist-disk/generated_test.go index eb690aaa54bb..ec09ecdc7fc2 100644 --- a/pkg/sql/logictest/tests/fakedist-disk/generated_test.go +++ b/pkg/sql/logictest/tests/fakedist-disk/generated_test.go @@ -2172,6 +2172,13 @@ func TestLogic_udf_plpgsql( runLogicTest(t, "udf_plpgsql") } +func TestLogic_udf_prepare( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_prepare") +} + func TestLogic_udf_privileges( 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 617a91b7a2cb..8e6b1dce4ca9 100644 --- a/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go +++ b/pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go @@ -2179,6 +2179,13 @@ func TestLogic_udf_plpgsql( runLogicTest(t, "udf_plpgsql") } +func TestLogic_udf_prepare( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_prepare") +} + func TestLogic_udf_privileges( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/fakedist/generated_test.go b/pkg/sql/logictest/tests/fakedist/generated_test.go index 0f3f3ee01a80..46de667d0de3 100644 --- a/pkg/sql/logictest/tests/fakedist/generated_test.go +++ b/pkg/sql/logictest/tests/fakedist/generated_test.go @@ -2193,6 +2193,13 @@ func TestLogic_udf_plpgsql( runLogicTest(t, "udf_plpgsql") } +func TestLogic_udf_prepare( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_prepare") +} + func TestLogic_udf_privileges( 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 757a834a42dc..9629f5d5f9e9 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 @@ -2172,6 +2172,13 @@ func TestLogic_udf_plpgsql( runLogicTest(t, "udf_plpgsql") } +func TestLogic_udf_prepare( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_prepare") +} + func TestLogic_udf_privileges( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/local-mixed-22.2-23.1/generated_test.go b/pkg/sql/logictest/tests/local-mixed-22.2-23.1/generated_test.go index 744cce7f460b..f0a364df427f 100644 --- a/pkg/sql/logictest/tests/local-mixed-22.2-23.1/generated_test.go +++ b/pkg/sql/logictest/tests/local-mixed-22.2-23.1/generated_test.go @@ -2109,6 +2109,13 @@ func TestLogic_udf_plpgsql( runLogicTest(t, "udf_plpgsql") } +func TestLogic_udf_prepare( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_prepare") +} + func TestLogic_udf_privileges( 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 196435026dde..1239431382e6 100644 --- a/pkg/sql/logictest/tests/local-vec-off/generated_test.go +++ b/pkg/sql/logictest/tests/local-vec-off/generated_test.go @@ -2200,6 +2200,13 @@ func TestLogic_udf_plpgsql( runLogicTest(t, "udf_plpgsql") } +func TestLogic_udf_prepare( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_prepare") +} + func TestLogic_udf_privileges( t *testing.T, ) { diff --git a/pkg/sql/logictest/tests/local/generated_test.go b/pkg/sql/logictest/tests/local/generated_test.go index e330504a1aa4..df4ed432d094 100644 --- a/pkg/sql/logictest/tests/local/generated_test.go +++ b/pkg/sql/logictest/tests/local/generated_test.go @@ -2396,6 +2396,13 @@ func TestLogic_udf_plpgsql( runLogicTest(t, "udf_plpgsql") } +func TestLogic_udf_prepare( + t *testing.T, +) { + defer leaktest.AfterTest(t)() + runLogicTest(t, "udf_prepare") +} + func TestLogic_udf_privileges( t *testing.T, ) { diff --git a/pkg/sql/sem/eval/expr.go b/pkg/sql/sem/eval/expr.go index 497dd6636a91..65df7098d8e8 100644 --- a/pkg/sql/sem/eval/expr.go +++ b/pkg/sql/sem/eval/expr.go @@ -476,6 +476,10 @@ func (e *evaluator) EvalFuncExpr(ctx context.Context, expr *tree.FuncExpr) (tree return tree.DNull, err } + if fn.Body != "" { + return nil, pgerror.Newf(pgcode.FeatureNotSupported, "cannot evaluate function in this context") + } + res, err := fn.Fn.(FnOverload)(ctx, e.ctx(), args) if err != nil { return nil, expr.MaybeWrapError(err)