-
Notifications
You must be signed in to change notification settings - Fork 439
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
[GLUTEN-7028][CH][Part-2] Refactor: Move MergeTree related UT to mergetree module #7279
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 |
b9a18ff
to
4f1adec
Compare
Run Gluten Clickhouse CI |
Run Gluten Clickhouse CI |
- Use CHConf - use CHConf.prefixOf() instead of "spark.gluten.sql.columnar.backend.ch." - settingsKey => runtimeSettings - configKey => runtimeConfig - CH => CONF_PREFIX
2d48984
to
c670f08
Compare
Run Gluten Clickhouse CI |
===== Performance report for TPCDS SF2000 with Velox backend, for reference only ====
|
===== Performance report for TPCH SF2000 with Velox backend, for reference only ====
|
…etree module (apache#7279) * Add CHConf * Move MergeTree related UT to mergetree module * fix scala stye * spark32 spark33 spark35 * More CH Conf * update per apache#7265 - Use CHConf - use CHConf.prefixOf() instead of "spark.gluten.sql.columnar.backend.ch." - settingsKey => runtimeSettings - configKey => runtimeConfig - CH => CONF_PREFIX * fix due to apache#7263
What changes were proposed in this pull request?
This PR does two things:
org.apache.gluten.execution.mergetree
CHConf.GlutenCHConf
, wo can simpfy setting configs, see the following pigture.Other
GlutenClickHouseMergeTreeCacheDataSuite
,GlutenClickHouseMergeTreeWriteOnHDFSSuite
,GlutenClickHouseMergeTreeWriteOnHDFSWithRocksDBMetaSuite
andGlutenClickHouseMergeTreeWriteOnS3Suite
, previously, we use default value '/' which is bad for local test.(Fixes: #7028)
How was this patch tested?
Using Existed UTs