Hi, I'd like to bring attention to this again. As you know, the hive-metastore module (which provides the HiveCatalog) is built against Hive 2. I have a PR, https://github.com/apache/iceberg/pull/12721, for building and testing the hive-metastore module against Hive 2, 3 and 4. Daniel Weeks looked into building the module against a single version of Hive and testing that against all three Hive versions, but this approach turned out to be infeasible because of binary incompatibility introduced by Hive 4 (so e.g., a hive-metastore jar built against Hive 2 would be incompatible if we use Hive 4 jars for our Hive dependencies). Note that Hive 4 also breaks thrift compatibility, so talking to a Hive 4 HMS requires using a Hive 4 client.
Given this, I propose that we have three versions of the module, hive-metastore, hive3-metastore and hive4-metastore, built against Hive 2, 3 and 4 respectively. (This is currently what is implemented in the PR.) This means three versions of the artifact. First, I'd like to get agreement from the community on doing this. Second, the hive-metastore classes are currently bundled in runtime jars, e.g., the Spark runtime jar and the Flink runtime jar. If we have agreement on the first point, I'd like to hear opinions on options for this second point: a. Remove hive-metastore classes from these runtime jars. Users will pick the hive-metastore jar appropriate to the Hive version they are using and add it to the classpath of their engine. b. Keep the runtime jars unchanged, which means bundling the hive-metastore classes built against Hive 2. c. Build multiple versions of the runtime jars, for Hive versions supported by those engines. (This does not mean all combinations, as e.g., Spark 3 does not support Hive 4.) a. provides the most flexibility at the expense of inconvenience to the user (to have to add another jar to the classpath). c. is the most complex. There is a separate question of testing engines such as Spark and Flink against different versions of HMS. At this time, I do not wish to get into this question until the above is resolved. Regardless of the option chosen for the second point, I think it is possible to test engines against different versions of HMS. Thanks, Wing Yew On Fri, Apr 11, 2025 at 12:03 PM Daniel Weeks <dwe...@apache.org> wrote: > Hey Wing, > > There are a few concerning issues I have with the current PR: > > 1) We need to update LICENSE/NOTICE for the hive3/hive4 dependencies > because I believe we only looked at the referenced version > 2) We're producing artifacts for hive3 and hive4 modules which I think we > want to exclude (we shouldn't triplicate the artifacts that are cross > compatible) > 3) I don't think we should be copying the hive DDL files into the Iceberg > project > > I'd really like to find a workaround for #3. I've added comments to the > PR as well. > > Thanks, > Dan > > On Fri, Apr 11, 2025 at 9:28 AM Wing Yew Poon <wyp...@cloudera.com.invalid> > wrote: > >> Hi, >> >> I created https://github.com/apache/iceberg/pull/12721 as an alternative >> to my earlier PR, https://github.com/apache/iceberg/pull/12681. With >> very helpful pointers from Peter Vary and his persistent prodding, we have >> an implementation where a single source set can be used to build and test >> with Hive 2, 3 and 4. >> >> In the new PR, the existing hive-metastore module is still built with >> Hive 2, and additional modules, hive3-metastore and hive4-metastore are >> introduced. Currently no other modules depend on the latter two. All three >> are built and tested in the existing java-ci workflow (without any change). >> >> The (non-test) classes in the hive-metastore module are packaged in the >> Flink and Spark runtime JARs. As a single source is used to support Hive 2, >> 3 and 4, they can be used with a HiveCatalog backed by a Hive 2, 3 or 4 >> HMS. In the PR, we do not make any changes to Flink or Spark modules. >> Testing them with a different version of Hive is the next step. I'd like to >> hear your opinion on how to approach that. >> >> Thanks, >> Wing Yew >> >> >> >> >> On Thu, Apr 3, 2025 at 5:31 PM Wing Yew Poon <wyp...@cloudera.com> wrote: >> >>> Hi Peter, >>> >>> Thank you for your thoughts on this. >>> >>> My PR, https://github.com/apache/iceberg/pull/12681, achieves the >>> following objectives: >>> (1) Build and test the hive-metastore module against Hive 2, 3, and 4. >>> (As you suggested in the earlier thread, we build and test against 2.3.10, >>> 3.1.3, and 4.0.1.) >>> (2) Keep a single code source for the hive-metastore implementation (the >>> test code needed to be different, unfortunately). >>> >>> In order to achieve (2), code changes were in fact needed (due to >>> HIVE-27925 <https://issues.apache.org/jira/browse/HIVE-27925> breaking >>> backward compatibility). So, even if HMS API is backwards compatible (and >>> HiveConf is not considered to be a part of HMS API), this shows the >>> importance of actually doing (1). >>> As a result of (2), even though in this PR, Flink and Spark modules >>> continue to be built with Hive 2 dependencies, the hive-metastore classes >>> packaged in the Flink and Spark runtime jars are usable with Hive 2, 3, or >>> 4 HMS (provided the engine supported that version of Hive). >>> >>> The use of separate Gradle modules (hive3-metastore, hive4-metastore) is >>> an implementation detail. I don't think we disagree on the objectives. If >>> you have suggestions for implementing the objectives differently, I'm happy >>> to hear them. To avoid getting into the weeds, we can discuss them in the >>> PR. >>> >>> I understand the desire to have a single version of `TestHiveMetastore`, >>> a test class that is used by the Flink and Spark modules as well. >>> Unfortunately, I did not see a way of doing this even with >>> `DynConstructors` and `DynMethods`. In Hive 4, `HMSHandler` became a >>> top-level class instead of being a nested class in `HiveMetaStore` (and I >>> don't see how to import one class from one version of Hive and a different >>> class from another version of Hive). Even with `DynConstructors` you have >>> to give it the instance of the class. Again, if you have suggestions to >>> overcome this, we can discuss them in the PR. However, I feel that keeping >>> a single version of test code that works for all Hive versions, while >>> desirable, should not be a blocker. >>> >>> The scope of the PR is deliberately limited to the hive-metastore >>> module. I feel that what versions of Hive to use for testing Flink and >>> Spark modules against the HiveCatalog and how to do so should be a second >>> (and possibly even third) step. As an experiment/proof of concept, in >>> https://github.com/apache/iceberg/pull/12693, I show that Flink modules >>> run successfully with hive-metastore built with Hive 4. Additionally, Spark >>> modules run successfully with hive-metastore built with Hive 3 (they fail >>> with Hive 4). I would say though that while Spark 4.0 will support using a >>> Hive 4 HMS, I do not expect we will see Spark 3.4 or 3.5 supporting using a >>> Hive 4 HMS. Thus, I do not think we should block this work on what Spark >>> supports. >>> >>> Thanks, >>> Wing Yew >>> >>> >>> >>> >>> On Thu, Apr 3, 2025 at 3:01 AM Péter Váry <peter.vary.apa...@gmail.com> >>> wrote: >>> >>>> Hi Wing Yew, >>>> >>>> Thanks for taking a look at this. >>>> After the removal of the Hive runtime code, we only depend on HMS in >>>> the HiveCatalog module in the production code. Since HMS API is supposed to >>>> be backward compatible, I would prefer to keep a single hive-metastore >>>> module and a single source dir for accessing HMS. When there is need we can >>>> use the DynMethods as we do now. >>>> >>>> I understand that for testing we need to start a HiveMetastore. It is >>>> used by Flink and Spark as well. Here things become harder as we need to >>>> instantiate a test Hive Metastore where we don't have backward >>>> compatibility guarantees. If possible, I would still prefer to keep a >>>> single codebase for testing purposes where we use DynMethods and >>>> HiveVersion to decide which version of HMS we instantiate. This could >>>> easily become a mess, but I would try to check if we can get away with it >>>> somehow. >>>> >>>> I would definitely run the hive-metastore tests with all of the >>>> supported Hive versions. OTOH, I would defer to the Spark/Flink maintainers >>>> to decide how confident they are in their products ability to abstract away >>>> the differences between the Hive versions. For Flink I would suggest to run >>>> the Flink tests only with the latest supported Hive version. >>>> >>>> Thanks, >>>> Peter >>>> >>>> >>>> Wing Yew Poon <wyp...@cloudera.com.invalid> ezt írta (időpont: 2025. >>>> márc. 31., H, 4:27): >>>> >>>>> Hi, >>>>> >>>>> I want to follow up on the earlier discussion about Hive support, >>>>> where it was decided to remove the Hive runtime support from the Iceberg >>>>> repo, and leave the Hive metastore support (where the HiveCatalog is >>>>> implemented) on Hive 2 for the time being. >>>>> >>>>> In that earlier thread, Peter Vary proposed that we support Hive >>>>> 2.3.10, 3.1.3 and 4.0.1 for the hive-metastore module and ensure that it >>>>> gets built and tested against those versions. >>>>> >>>>> I have implemented this in >>>>> https://github.com/apache/iceberg/pull/12681. I have left the >>>>> existing hive-metastore module depending on Hive 2.3.10, and added new >>>>> hive3-metastore and hive4-metastore modules that depend on Hive 3.1.3 and >>>>> 4.0.1 respectively. I have followed the approach used by the mr and hive3 >>>>> modules previously and kept all common code in one directory (the existing >>>>> hive-metastore directory) and avoided code duplication. In order to >>>>> workaround https://issues.apache.org/jira/browse/HIVE-27925, which >>>>> introduced a backward incompatibility in Hive 4, I have avoided the use of >>>>> HiveConf.ConfVars enums and used the conf property names (which have not >>>>> changed) instead. (This is also the approach used by Spark in >>>>> https://issues.apache.org/jira/browse/SPARK-47679.) Please see the PR >>>>> for more details. >>>>> >>>>> The Flink and Spark modules (along with the delta-lake module) have >>>>> test code that depend on the hive-metastore module as well as on Hive >>>>> metastore jars. Having those modules test against Hive 3 and Hive 4 >>>>> metastore versions is not in the scope of the above PR. I plan to work on >>>>> that separately as follow up, and I want to hear opinions on the approach. >>>>> As a proof of concept, I have put up >>>>> https://github.com/apache/iceberg/pull/12693 with follow on changes >>>>> to test the Flink modules against hive4-metastore. This is straightforward >>>>> and there are no issues. >>>>> >>>>> For Spark though, as I have mentioned in the earlier thread, Spark >>>>> uses a built-in version of the Hive metastore (currently 2.3.10), but can >>>>> be configured to use a different version and be pointed to a path >>>>> containing Hive metastore jars for the different version. However, the >>>>> highest Hive version that can be configured for Spark 3.5 is 3.1.3 (Spark >>>>> 4 >>>>> will support 4.0.x), as changes in Spark code is needed to be able to >>>>> workaround HIVE-27925. >>>>> >>>>> What I'm interested in hearing is: For testing Flink and Spark against >>>>> Hive versions, do we want to test against >>>>> (1) just one version, e.g., the highest version supportable by that >>>>> Flink/Spark version (or alternatively just 2.3.10). >>>>> (2) multiple versions from 2.3.10, 3.1.3 and 4.0.1, as long as they >>>>> are supportable by that Flink/Spark version. >>>>> And if (2), how do we want to do that, e.g., full matrix, or some kind >>>>> of sampling. >>>>> >>>>> Thanks, >>>>> Wing Yew >>>>> >>>>>
