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

[CH-358]Support json_tuple function #361

Merged

Conversation

KevinyhZou
Copy link

@KevinyhZou KevinyhZou commented Mar 15, 2023

Changelog category (leave one):

  • New Feature

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Implement json_tuple funtion by use JSONExtract function in ck, the releated issue: #358

  1. convert json_tuple to JSONExtract function;
  2. convert json_tuple result to multi columns likes spark json_tuple

@kyligence-git
Copy link
Collaborator

Can one of the admins verify this patch?

@liuneng1994
Copy link
Collaborator

test this please

@zzcclp
Copy link
Collaborator

zzcclp commented Mar 17, 2023

please add a gluten pr to add some ut for this function, thanks

@KevinyhZou
Copy link
Author

please add a gluten pr to add some ut for this function, thanks

ok

@KevinyhZou
Copy link
Author

please add a gluten pr to add some ut for this function, thanks

ut added done in gluten pr: apache/incubator-gluten#1135. Thanks for your review @zzcclp

@liuneng1994
Copy link
Collaborator

test this please

@liuneng1994
Copy link
Collaborator

test this please with 1135

1 similar comment
@liuneng1994
Copy link
Collaborator

test this please with 1135

@@ -700,7 +700,7 @@ QueryPlanPtr SerializedPlanParser::parse(std::unique_ptr<substrait::Plan> plan)
ActionsDAGPtr actions_dag = std::make_shared<ActionsDAG>(blockToNameAndTypeList(query_plan->getCurrentDataStream().header));
NamesWithAliases aliases;
auto cols = query_plan->getCurrentDataStream().header.getNamesAndTypesList();
for (int i = 0; i < root_rel.root().names_size(); i++)
for (int i = 0; i < cols.getNames().size(); i++)
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里有编译警告

Copy link
Author

Choose a reason for hiding this comment

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

Fix done. @liuneng1994

@liuneng1994
Copy link
Collaborator

test this please with 1135

@KevinyhZou
Copy link
Author

test this please with 1135

@KevinyhZou
Copy link
Author

@liuneng1994 cc

Copy link
Collaborator

@liuneng1994 liuneng1994 left a comment

Choose a reason for hiding this comment

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

LGTM

@liuneng1994 liuneng1994 merged commit 2b37553 into Kyligence:clickhouse_backend Apr 13, 2023
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