HIVE-27925: HiveConf: unify ConfVars enum and use underscore for better readability #4919
Conversation
asf-ci-hive
added
tests pending
tests failed
and removed
tests failed
tests pending
labels
kokila-19
force-pushed
the
HIVE-27925
branch
from
1220d10 to
cd2cb8a
Compare
asf-ci-hive
added
tests pending
tests failed
and removed
tests failed
tests pending
labels
kokila-19
force-pushed
the
HIVE-27925
branch
from
cd2cb8a to
a3456bc
Compare
asf-ci-hive
added
tests pending
tests unstable
and removed
tests failed
tests pending
labels
There was a problem hiding this comment.
+1 it is just a rename refactor which is nice
+1 for @abstractdog for this detailed describtion:
When I read something like "BASICSTATSTASKSMAXTHREADSFACTOR" I feel someone in the world laughs out loud thinking of me struggling. I can read it, but I hate it imagine what if we have vars like HIVE_MATERIALIZED_VIEW_ENABLE_AUTO_REWRITING_SUBQUERY_SQL without underscores...okay, let me help, it is: HIVEMATERIALIZEDVIEWENABLEAUTOREWRITINGSUBQUERYSQL
Made my day :D
btw: This is a real and meaningful Hungarian word: megszentségteleníthetetlenségeskedéseitekért (same as our enums :) useless)
|
@ayushtkn @abstractdog @deniskuzZ Could you please check this PR as well. Many files are touched but it is a simple rename. |
kokila-19
force-pushed
the
HIVE-27925
branch
from
a3456bc to
f273fe2
Compare
asf-ci-hive
added
tests pending
tests unstable
and removed
tests failed
tests pending
labels
|
…er readability (apache#4919) (Kokila N reviewed by Laszlo Bodor)
…er readability (apache#4919) (Kokila N reviewed by Laszlo Bodor)
…ctly ### What changes were proposed in this pull request? This PR aims to use `HiveConf.getConfVars` or Hive config names directly to be robust on Hive incompatibility. ### Why are the changes needed? Apache Hive 4.0.0 introduced incompatible changes on `ConfVars` enum via HIVE-27925. - apache/hive#4919 - apache/hive#5107 `HiveConf.getConfVars` or config names is more robust way to handle this incompatibility. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #45804 from dongjoon-hyun/SPARK-47679. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
|
This change seems to break the compatibility with apache.iceberg.hive dependency: We have been using the HiveCatalog of apache.iceberg prior the newly introduced changes of 4.0.0 Alpha 1. With this change, this isn't possible anymore. Furthermore, if I switch to apache.hive HiveCatalog then the necessary included shading of fasterxml breaks everything. Because the JsonUtil class in apache.iceberg includes the fasterxml dependency without the relocated path. Anyone an idea what to do here? Here is my feature request for apache.iceberg to introduce the ENV changes there too: apache/iceberg#10254 Here is my jira issue where I specified the current problem I have with the shading: Thanks! |
|
If using Hive > 4.0.0 then use the iceberg shading dependency. All good here. Probably could be added to the ReadMe ;) |
What changes were proposed in this pull request?
HiveConf: unify ConfVars enum and use underscore for better readability
Why are the changes needed?
Better readability
Does this PR introduce any user-facing change?
No
Is the change a dependency upgrade?
No
How was this patch tested?
Build and Unit tests.