-
Notifications
You must be signed in to change notification settings - Fork 447
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] Use collection interface in method parameter and return type #3603
Conversation
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?
See also: |
Run Gluten Clickhouse CI |
b538196
to
acfa42b
Compare
Run Gluten Clickhouse CI |
1 similar comment
Run Gluten Clickhouse CI |
import java.lang.{Long => JLong} | ||
import java.util.{HashMap => JHashMap} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this import statement be generated by some kind of tools? Or the code was manually written?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made the modifications manually. However, upon carefully examining the code of Spark, I noticed that it actually has a lot of inconsistency in its usage. There are various conventions being used, with most of them using JList
, but java.util.List
and util.List
are still present as well. I am now questioning whether it is necessary to make these modifications here, as it would be difficult to ensure that all future developers maintain a consistent style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as it would be difficult to ensure that all future developers maintain a consistent style.
Yes so it's not necessary to become a coding style or restriction. Sounds fair to me.
Run Gluten Clickhouse CI |
1 similar comment
Run Gluten Clickhouse CI |
eaf3594
to
974d587
Compare
Run Gluten Clickhouse CI |
974d587
to
064260b
Compare
Run Gluten Clickhouse CI |
064260b
to
ad0c317
Compare
Run Gluten Clickhouse CI |
@zhztheplayer @rui-mo It's ready for review. I have removed many unnecessary import modifications. |
===== Performance report for TPCH SF2000 with Velox backend, for reference only ====
|
What changes were proposed in this pull request?
It is recommended to use abstract interfaces for method parameters and return type instead of concrete implementations.
After modifying the method parameters and return type to abstract interface types, many codes can be rewritten in a more functional style. This PR also includes these modifications as well.
How was this patch tested?
Existing test cases