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

opt: build UDF expressions #84723

Merged
merged 2 commits into from
Jul 25, 2022
Merged

opt: build UDF expressions #84723

merged 2 commits into from
Jul 25, 2022

Conversation

mgartner
Copy link
Collaborator

opt: set opt tester search path to empty

OptTester now sets its SemaContext's SearchPath to
EmptySearchPath, instead of nil, to avoid nil pointer exceptions
when resolving unknown functions.

Release note: None

opt: build UDF expressions

This commit adds basic support for building UDFs in optbuilder. Only
scalar, nullary (arity of zero) functions with a single statement in the
body are supported. Support for more types of UDFs will follow in future
commits. Note that this commit does not add support for execution of
UDFs, only building them within an optimizer expression.

Release note: None

@mgartner mgartner requested review from rytaft and michae2 July 20, 2022 16:20
@mgartner mgartner requested a review from a team as a code owner July 20, 2022 16:20
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@rytaft rytaft left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @michae2)


pkg/sql/opt/norm/decorrelate_funcs.go line 67 at r2 (raw file):

	case *memo.UserDefinedFunctionExpr:
		// Do not attempt to hoist UDFs.

Is this a limitation we'll be able to lift at some point?


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

	// A statement inside a UDF body cannot refer to anything from the outer
	// expression calling the function, so we use an empty scope.
	// TODO(mgartner): We may need ot set bodyScope.atRoot=true to prevent CTEs

nit: ot -> to

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.

TFTR!

bors r+

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @michae2 and @rytaft)


pkg/sql/opt/norm/decorrelate_funcs.go line 67 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

Is this a limitation we'll be able to lift at some point?

Yes, we'll want to inline UDFs when possible. I'm not positive yet whether that transformation will be part of these hoist-related rules or separate, but the goal of the transformation is similar.


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

Previously, rytaft (Rebecca Taft) wrote…

nit: ot -> to

Done.

@craig
Copy link
Contributor

craig bot commented Jul 25, 2022

Build failed:

@mgartner
Copy link
Collaborator Author

bors r-

@mgartner
Copy link
Collaborator Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jul 25, 2022

Build failed:

`OptTester` now sets its `SemaContext`'s `SearchPath` to
`EmptySearchPath`, instead of `nil`, to avoid nil pointer exceptions
when resolving unknown functions.

Release note: None
This commit adds basic support for building UDFs in optbuilder. Only
scalar, nullary (arity of zero) functions with a single statement in the
body are supported. Support for more types of UDFs will follow in future
commits. Note that this commit does not add support for execution of
UDFs, only building them within an optimizer expression.

Release note: None
Copy link
Collaborator

@michae2 michae2 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r1, 6 of 6 files at r4, 6 of 6 files at r5, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @rytaft)

@mgartner
Copy link
Collaborator Author

bors r+

@craig craig bot merged commit 9e44f56 into cockroachdb:master Jul 25, 2022
@craig
Copy link
Contributor

craig bot commented Jul 25, 2022

Build succeeded:

@mgartner mgartner deleted the udf-opt-build branch July 25, 2022 18:59
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.

None yet

4 participants