-
Notifications
You must be signed in to change notification settings - Fork 466
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
[CORE] Enlarge defaultRecursionLimit by pre-loading the protobuf class #8904
[CORE] Enlarge defaultRecursionLimit by pre-loading the protobuf class #8904
Conversation
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/apache/incubator-gluten/issues Then could you also rename commit message and pull request title in the following format?
See also: |
Run Gluten Clickhouse CI on x86 |
related |
incubator-gluten/gluten-iceberg/pom.xml Line 84 in 18f1510
set to "provide" |
incubator-gluten/package/pom.xml Line 356 in 18f1510
Remove? |
Run Gluten Clickhouse CI on x86 |
@FelixYBW, I just removed protobuf from iceberg & hudi module as it is not really used, and changed the scope to provided for delta module. |
Yes, seems we can remove it. Let's see CI feedback. |
Where is the code to modify protobuf and build customized version? Is it manually upload to mvn repo? |
@FelixYBW , the code is maintained in my personal repo: https://github.com/PHILO-HE/protobuf/tree/v3.23.4-0. Yes, we manually did that. |
Thank you for the PR! manually doing this definitely isn't a right way. Glad we fixed it. |
What changes were proposed in this pull request?
We previously introduced custom protobuf dependency with only defaultRecursionLimit enlarged to avoid the limit is hit for deeply nested plans (one user reported this issue). It is not easy to upgrade this dependency since it requires local build and then publishing the jar to maven repo.
With this change, we can remove the custom protobuf dependency for easily upgrading protobuf when needed.
How was this patch tested?
Verified by local remote debug.