Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: support UDFs with named args, strictness, and volatility #85786

Merged
merged 4 commits into from
Aug 12, 2022

Conversation

mgartner
Copy link
Collaborator

@mgartner mgartner commented Aug 8, 2022

sql: UDF with empty result should evaluate to NULL

If the last statement in a UDF returns no rows, the UDF will evaluate to
NULL. Prior to this commit the evaluation of the UDF would panic.

Release note: None

sql: support UDFs with named arguments

UDFs with named arguments can now be evaluated.

During query planning, statements in the function body are built with a
scope that includes the named arguments for the function as columns.
This allows references to arguments to be resolved as variables.

During evaluation, the input expressions are first evaluated into
datums. When a plan is built for each statement in the UDF, the argument
columns in the expression are replaced with the input datums before the
expression is optimized.

Note that anonymous arguments and integer references to arguments (e.g.,
$1) are not yet supported.

Also, the formatting of UDFExprs has been improved to show argument
columns and input expressions.

Release note: None

sql: do not evaluate strict UDFs if any input values are NULL

A UDF can have one of two behaviors when it is invoked with NULL inputs:

  1. If the UDF is CALLED ON NULL INPUT (the default) then the
    function is evaluated regardless of whether or not any of the input
    values are NULL.
  2. If the UDF RETURNS NULL ON NULL INPUT or is STRICT then the
    function is not evaluated if any of the input values are NULL.
    Instead, the function directly results in NULL.

This commit implements these two behaviors.

In the future, we can add a normalization rule that folds a strict UDF
if any of its inputs are constant NULL values.

Release note: None

sql: make mutations visible to volatile UDFs

The volatility of a UDF affects the visibility of mutations made by the
statement calling the function. A volatile function will see these
mutations. Also, statements within a volatile function's body will see
changes made by previous statements the function body (note that this is
left untested in this commit because we do not currently support
mutations within UDF bodies). In contrast, a stable, immutable, or
leakproof function will see a snapshot of the data as of the start of
the statement calling the function.

Release note: None

@mgartner mgartner requested review from a team as code owners August 8, 2022 23:49
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: probably would be good to wait for Drew to take a look too.

Reviewed 2 of 2 files at r1, 13 of 13 files at r2, 7 of 7 files at r3, 5 of 5 files at r4, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball and @mgartner)


pkg/sql/routine.go line 53 at r4 (raw file):

	txn := p.Txn()
	if expr.Volatility == volatility.Volatile {
		// _ = p.Txn().ConfigureStepping(ctx, kv.SteppingEnabled)

nit: leftover.


pkg/sql/opt/ops/scalar.opt line 1251 at r4 (raw file):

    # calling the function. A volatile function will see these mutations. Also,
    # statements within a volatile function's body will see changes made by
    # previous statements the function body. In contrast, a stable, immutable,

nit: probably s/statements the/statements in the/.


pkg/sql/opt/optbuilder/scalar.go line 612 at r2 (raw file):

		input = make(memo.ScalarListExpr, len(f.Exprs))
		for i, pexpr := range f.Exprs {
			input[i] = b.buildScalar(pexpr.(tree.TypedExpr), inScope, nil, nil, colRefs)

nit: add inlined comments for nils.


pkg/sql/sem/tree/routine.go line 57 at r4 (raw file):

	// invoking the routine. A volatile routine will see these mutations. Also,
	// statements within a volatile function's body will see changes made by
	// previous statements the routine. In contrast, a stable, immutable,

nit: ditto

Copy link
Collaborator

@DrewKimball DrewKimball left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained (waiting on @mgartner)


pkg/sql/logictest/testdata/logic_test/udf line 514 at r4 (raw file):

  get_v(k) v1, get_v(int_identity_v(k)) v2,
  get_i(k) i1, get_i(int_identity_v(k)) i2,
  get_l(k) l1, get_l(int_identity_v(k)) l2

[super nit] could you put the columns in the same order as the CREATE FUNCTION statements?


pkg/sql/opt/exec/execbuilder/testdata/udf line 82 at r4 (raw file):

# the function is evaluated. It shows the scan performed by the statement in the
# function body.
# that the scan within the function is performed.

[nit] looks like this is left over.

@mgartner mgartner force-pushed the udf-args-volatile-strict branch from 1b80e41 to d96cc93 Compare August 9, 2022 13:01
Copy link
Collaborator Author

@mgartner mgartner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick reviews!

Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained (waiting on @DrewKimball and @yuzefovich)


pkg/sql/routine.go line 53 at r4 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

nit: leftover.

Done.


pkg/sql/logictest/testdata/logic_test/udf line 514 at r4 (raw file):

Previously, DrewKimball (Drew Kimball) wrote…

[super nit] could you put the columns in the same order as the CREATE FUNCTION statements?

Done.


pkg/sql/opt/exec/execbuilder/testdata/udf line 82 at r4 (raw file):

Previously, DrewKimball (Drew Kimball) wrote…

[nit] looks like this is left over.

Good catch.


pkg/sql/opt/ops/scalar.opt line 1251 at r4 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

nit: probably s/statements the/statements in the/.

Good catch.


pkg/sql/opt/optbuilder/scalar.go line 612 at r2 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

nit: add inlined comments for nils.

Done.


pkg/sql/sem/tree/routine.go line 57 at r4 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

nit: ditto

done.

@mgartner mgartner force-pushed the udf-args-volatile-strict branch 2 times, most recently from 552e317 to 455b435 Compare August 10, 2022 16:24
@mgartner
Copy link
Collaborator Author

TFTRs!

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 11, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 11, 2022

Build failed:

@mgartner
Copy link
Collaborator Author

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 11, 2022

Build failed (retrying...):

@mgartner
Copy link
Collaborator Author

bors r-

@craig
Copy link
Contributor

craig bot commented Aug 11, 2022

Canceled.

If the last statement in a UDF returns no rows, the UDF will evaluate to
NULL. Prior to this commit the evaluation of the UDF would panic.

Release note: None
UDFs with named arguments can now be evaluated.

During query planning, statements in the function body are built with a
scope that includes the named arguments for the function as columns.
This allows references to arguments to be resolved as variables.

During evaluation, the input expressions are first evaluated into
datums. When a plan is built for each statement in the UDF, the argument
columns in the expression are replaced with the input datums before the
expression is optimized.

Note that anonymous arguments and integer references to arguments (e.g.,
`$1`) are not yet supported.

Also, the formatting of `UDFExpr`s has been improved to show argument
columns and input expressions.

Release note: None
A UDF can have one of two behaviors when it is invoked with NULL inputs:

  1. If the UDF is `CALLED ON NULL INPUT` (the default) then the
     function is evaluated regardless of whether or not any of the input
     values are NULL.
  2. If the UDF `RETURNS NULL ON NULL INPUT` or is `STRICT` then the
     function is not evaluated if any of the input values are NULL.
     Instead, the function directly results in NULL.

This commit implements these two behaviors.

In the future, we can add a normalization rule that folds a strict UDF
if any of its inputs are constant NULL values.

Release note: None
The volatility of a UDF affects the visibility of mutations made by the
statement calling the function. A volatile function will see these
mutations. Also, statements within a volatile function's body will see
changes made by previous statements the function body (note that this is
left untested in this commit because we do not currently support
mutations within UDF bodies). In contrast, a stable, immutable, or
leakproof function will see a snapshot of the data as of the start of
the statement calling the function.

Release note: None
@mgartner mgartner force-pushed the udf-args-volatile-strict branch from 455b435 to c4bf42a Compare August 11, 2022 16:31
@mgartner
Copy link
Collaborator Author

bors r+

@mgartner
Copy link
Collaborator Author

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 11, 2022

Already running a review

@mgartner
Copy link
Collaborator Author

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 11, 2022

Already running a review

@craig
Copy link
Contributor

craig bot commented Aug 12, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 12, 2022

Build succeeded:

@craig craig bot merged commit e17eb36 into cockroachdb:master Aug 12, 2022
@mgartner mgartner deleted the udf-args-volatile-strict branch August 12, 2022 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants