Skip to content

Always evaluate built-in functions using the presto.default namespace #25135

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tdcmeehan
Copy link
Contributor

@tdcmeehan tdcmeehan commented May 16, 2025

Description

When the sidecar is used for query analysis, the default namespace is switched to reflect the different set of functions used in Velox. While eventually we wish to use Velox for both expression evaluation and also as a registry of functions, the former is taking a long time due to refactoring in Velox. In the meantime, we need function evaluation over C++ namespaces to work properly, which means we need to use the Java functions for now. This is a stopgap solution until function evaluation in Velox works end to end (tracked by #24238).

Motivation and Context

See above.

Impact

This allows constant folding to work while using Velox functions for analysis with the sidecar.

Test Plan

Tests have been added.

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

Prestissimo (Native Execution)
* Use Java functions for constant folding when native execution is enabled.

@prestodb-ci prestodb-ci added the from:IBM PR from IBM label May 16, 2025
@tdcmeehan tdcmeehan force-pushed the constant_old branch 2 times, most recently from 2ea87fe to ca65c3a Compare May 19, 2025 13:53
This is needed to have consistent constant folding behavior across the codebase.
@tdcmeehan tdcmeehan marked this pull request as ready for review May 20, 2025 20:23
@tdcmeehan tdcmeehan requested review from shrinidhijoshi and a team as code owners May 20, 2025 20:23
@tdcmeehan tdcmeehan requested a review from ZacBlanco May 20, 2025 20:23
@prestodb-ci prestodb-ci requested review from a team, bibith4 and wanglinsong and removed request for a team May 20, 2025 20:23
Copy link
Contributor

@pdabre12 pdabre12 left a comment

Choose a reason for hiding this comment

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

@tdcmeehan Thanks for the changes.
Can we add an e2e test in TestNativeSidecarPlugin to test this?

If the user has no specified a custom expression optimizer, expressions should be
presumed to be in the presto.default namespace (which consists exclusively of Java
functions).
@tdcmeehan tdcmeehan requested a review from a team as a code owner May 23, 2025 19:55
@tdcmeehan tdcmeehan requested a review from pdabre12 May 23, 2025 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
from:IBM PR from IBM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants