Skip to content
This repository has been archived by the owner on Sep 18, 2023. It is now read-only.

[NSE-1019] fix codegen for all expressions #1039

Merged
merged 10 commits into from
Sep 21, 2022

Conversation

zhouyuan
Copy link
Collaborator

@zhouyuan zhouyuan commented Jul 20, 2022

What changes were proposed in this pull request?

This patch adds supportColumnarCodegen for all functions

  • by default all functions are set to false (not supporting codegen)
  • all codegen supported functions are marked as true explicitly

Signed-off-by: Yuan Zhou [email protected]

How was this patch tested?

pass jenkins

@github-actions
Copy link

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/oap-project/native-sql-engine/issues

Then could you also rename commit message and pull request title in the following format?

[NSE-${ISSUES_ID}] ${detailed message}

See also:

@PHILO-HE
Copy link
Collaborator

It looks ColumnarFromUnixTime also has a similar issue.

@zhouyuan
Copy link
Collaborator Author

It looks ColumnarFromUnixTime also has a similar issue.

let me see if we could mark the default codegen support as false

@zhouyuan zhouyuan changed the title fix codegen for unixtimestamp [NSE-1019] fix codegen for unixtimestamp Aug 5, 2022
@zhouyuan zhouyuan marked this pull request as ready for review August 5, 2022 05:28
@github-actions
Copy link

github-actions bot commented Aug 5, 2022

#1019

Signed-off-by: Yuan Zhou <[email protected]>
@zhouyuan zhouyuan changed the title [NSE-1019] fix codegen for unixtimestamp [NSE-1019] fix codegen for all expressions Aug 8, 2022
"get_json_object",
"translate",
"substr",
"instr",
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like instr should be replaced by locate here and in expression_codegen_visitor.cc. Please refer to the code in ColumnarStringInstr.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed in 7f38a8e

@zhouyuan zhouyuan merged commit 86e1a91 into oap-project:main Sep 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants