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] Fix exception of pb MessageToJsonString #3823

Merged
merged 1 commit into from
Nov 28, 2023
Merged

Conversation

exmy
Copy link
Contributor

@exmy exmy commented Nov 23, 2023

What changes were proposed in this pull request?

The advanced_extension part of the substrait plan for Join operator is like

advanced_extension {
    optimization {
    type_url: "/google.protobuf.StringValue"
    value: "\ncJoinParameters:isBHJ=1\nisNullAwareAntiJoin=0\nbuildHashTableId=BuiltHashTable-774\nisExistenceJoin=0\n"
    }
    enhancement {
    type_url: "/substrait.Type"
    value: "\312\0012\n\004:\002\020\001\n\004b\002\020\001\n\004Z\002\020\001\n\004Z\002\020\001\n\004Z\002\020\001\n\004:\002\020\001\n\004b\002\020\002\n\004b\002\020\001\030\002"
    }
}

The c++ protobuf version of CH backend had upgraded since #3756 , casuing the method of google::protobuf::util::MessageToJsonString converting the substrait plan to json failed due to INVALID_ARGUMENT: @type must contain at least one / and a nonempty host; got: /google.protobuf.StringValue.

How was this patch tested?

manual tests

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/gluten/issues

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

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

Copy link

Run Gluten Clickhouse CI

1 similar comment
Copy link

Run Gluten Clickhouse CI

Copy link

Run Gluten Clickhouse CI

Copy link

Run Gluten Clickhouse CI

@exmy exmy changed the title [CORE] Fix exception of pb MessageToJsonString [CH] Fix exception of pb MessageToJsonString Nov 24, 2023
@exmy
Copy link
Contributor Author

exmy commented Nov 24, 2023

@baibaichen @PHILO-HE Please take a review, thanks!

Copy link
Contributor

@PHILO-HE PHILO-HE left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@baibaichen baibaichen left a comment

Choose a reason for hiding this comment

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

LGTM

@baibaichen baibaichen merged commit 8518680 into apache:main Nov 28, 2023
19 checks passed
@GlutenPerfBot
Copy link
Contributor

===== Performance report for TPCH SF2000 with Velox backend, for reference only ====

query log/native_3823_time.csv log/native_master_11_27_2023_08cdbee7e_time.csv difference percentage
q1 34.75 33.96 -0.793 97.72%
q2 24.90 24.50 -0.401 98.39%
q3 37.96 36.70 -1.261 96.68%
q4 37.29 36.44 -0.846 97.73%
q5 71.89 70.50 -1.393 98.06%
q6 8.10 7.29 -0.803 90.09%
q7 84.72 83.48 -1.241 98.53%
q8 87.86 88.10 0.241 100.27%
q9 128.31 122.04 -6.272 95.11%
q10 45.08 46.95 1.869 104.15%
q11 19.73 19.82 0.086 100.44%
q12 25.98 26.08 0.102 100.39%
q13 46.13 48.07 1.940 104.21%
q14 18.26 15.26 -3.002 83.56%
q15 29.73 28.91 -0.819 97.24%
q16 16.66 15.89 -0.762 95.42%
q17 103.97 103.54 -0.437 99.58%
q18 150.50 151.52 1.019 100.68%
q19 14.45 12.96 -1.489 89.69%
q20 27.35 28.70 1.357 104.96%
q21 223.66 226.06 2.402 101.07%
q22 13.20 13.19 -0.006 99.95%
total 1250.47 1239.96 -10.510 99.16%

kecookier pushed a commit to kecookier/gluten that referenced this pull request May 20, 2024
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