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

[native pos] Remove workaround of isNative flag passed to SqlToRowExpressionTranslator #20008

Closed
vermapratyush opened this issue Jun 27, 2023 · 1 comment · Fixed by #22158
Closed
Assignees
Labels
pos Presto-on-Spark related prestissimo Presto Native Execution

Comments

@vermapratyush
Copy link
Member

vermapratyush commented Jun 27, 2023

isNative flag passed to SqlToRowExpressionTranslator from SqlFunctionUtils is hard coded to false https://github.com/vermapratyush/presto/blob/d5eed561c31e21cb0b6cfa5f94038d9d4f2b4b89/presto-main/src/main/java/com/facebook/presto/sql/relational/SqlFunctionUtils.java#L111
https://github.com/prestodb/presto/blob/d5eed561c31e21cb0b6cfa5f94038d9d4f2b4b89/presto-main/src/main/java/com/facebook/presto/sql/relational/SqlFunctionUtils.java#L111

Ideally, this should be derived from session value, hard coding this to false does not break any flow since we always disable inline sql function for native:
https://github.com/prestodb/presto/blob/master/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/NativeQueryRunnerUtils.java#L42

@vermapratyush
Copy link
Member Author

For SqlFunctionUtils,

the solution would be to add the isNative flag to the connector session and pass it through when we create the connector session.

@tdcmeehan tdcmeehan added prestissimo Presto Native Execution pos Presto-on-Spark related labels Jul 27, 2023
@mbasmanova mbasmanova self-assigned this Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pos Presto-on-Spark related prestissimo Presto Native Execution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants