You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "wypoon (via GitHub)" <gi...@apache.org> on 2025/03/29 01:40:50 UTC
[PR] Ignore. Testing WIP. [iceberg]
wypoon opened a new pull request, #12681:
URL: https://github.com/apache/iceberg/pull/12681
xxx
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2030390565
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java:
##########
@@ -303,10 +303,10 @@ public void testThriftExceptionConcurrentCommit() throws TException, Interrupted
@Test
public void testInvalidObjectException() {
- TableIdentifier badTi = TableIdentifier.of(DB_NAME, "`tbl`");
+ TableIdentifier badTi = TableIdentifier.of(DB_NAME, "tábl");
Review Comment:
Is this test still for hive2?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#issuecomment-2784870391
@manuzhang thanks for reviewing!
I'd prefer not to follow the approach taken by Spark and Flink, to have versioned directories with duplicated code. I'm trying to keep a single version of code all in one directory as far as possible. The model is more the old mr and hive3 directories.
If you mean keep versioned directories under hive-metastore, but do not duplicate code unless necessary, do you suggest a directory for common? E.g.,
```
hive-metastore/common
hive-metastore/v2
hive-metastore/v3
hive-metastore/v4
```
The idea has some appeal, but I didn't do this as I was trying to avoid too much disruption to the existing code base.
It seems a lot more moving files around without substantial benefit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#issuecomment-2787975659
@manuzhang thanks for your feedback.
@pvary was very keen to try to keep a single code base if possible. Please take a look at https://github.com/apache/iceberg/pull/12721. After some iterations with Peter, I have now implemented that objective. So now all the code and resources are in the hive-metastore directory.
I'm still open to the idea of having builds under
```
hive-metastore/v2
hive-metastore/v3
hive-metastore/v4
```
and there is probably some clever way to do that in gradle. Unfortunately, my knowledge of gradle is limited. That can also be a follow-up nice-to-do.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2023418469
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -445,7 +444,7 @@ public void testCreateTableCustomSortOrder() throws Exception {
@Test
public void testDatabaseAndNamespaceWithLocation() throws Exception {
- Namespace namespace1 = Namespace.of("noLocation");
+ Namespace namespace1 = Namespace.of("nolocation");
Review Comment:
difference is probably because Hive doesn't normally use `HiveCatalog#createNamespace`, only `HMS RestCatalog` does
````
database.setLocationUri(new Path(getExternalWarehouseLocation(), namespace.level(0)).toString() + ".db");
````
HMS
````
private String dbDirFromDbName(Database db) throws MetaException {
return db.getName().toLowerCase() + DATABASE_WAREHOUSE_SUFFIX;
}
````
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#issuecomment-2774990865
Hi @wypoon,
Thanks for taking a look at this. I really would like to see HiveCatalog testing against Hive4, but Spark has constraints around testing newer Hive versions which we need to solve, or wait until all of the supported Spark version will have an updated Hive.
OTOH, if possible, I would like to see only a single TestHiveMetastore class. Could we do some changes internally to the class, which based on the classes available on the classpath, or some configuration would use the correct Hive versions (maybe with DynMethods)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2023308361
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -445,7 +444,7 @@ public void testCreateTableCustomSortOrder() throws Exception {
@Test
public void testDatabaseAndNamespaceWithLocation() throws Exception {
- Namespace namespace1 = Namespace.of("noLocation");
+ Namespace namespace1 = Namespace.of("nolocation");
Review Comment:
With `Namespace.of("noLocation")`, when `HiveCatalog::createNamespace` is called, the metastore client calls `createDatabase` with a `Database` containing name "noLocation and locationUri ending with "noLocation.db", but when the client calls `getDatabase`, it gets back a `Database` containing name "nolocation" and locationUri ending with "nolocation.db". On disk, however, I can see that a directory "noLocation.db" has been created in the warehouse directory.
@pvary @deniskuzZ could you shed light on what changed in Hive 4 that causes this behavior? Is it anything we need to do something about on the Iceberg side?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2023379656
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -445,7 +444,7 @@ public void testCreateTableCustomSortOrder() throws Exception {
@Test
public void testDatabaseAndNamespaceWithLocation() throws Exception {
- Namespace namespace1 = Namespace.of("noLocation");
+ Namespace namespace1 = Namespace.of("nolocation");
Review Comment:
I need to check, however, as I recall Hive lower-cases all identifiers: db, table, partition
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2020290971
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java:
##########
@@ -303,10 +303,10 @@ public void testThriftExceptionConcurrentCommit() throws TException, Interrupted
@Test
public void testInvalidObjectException() {
- TableIdentifier badTi = TableIdentifier.of(DB_NAME, "`tbl`");
+ TableIdentifier badTi = TableIdentifier.of(DB_NAME, "tábl");
Review Comment:
Before Hive 4, the only special character allowed is '/':
https://github.com/apache/hive/blob/branch-3.1/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java#L155
In Hive 4, all non-alphanumeric characters in a US keyboard, including backtick ('`'), are allowed special characters:
https://github.com/apache/hive/blob/master/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java#L175-L181
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2023379656
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -445,7 +444,7 @@ public void testCreateTableCustomSortOrder() throws Exception {
@Test
public void testDatabaseAndNamespaceWithLocation() throws Exception {
- Namespace namespace1 = Namespace.of("noLocation");
+ Namespace namespace1 = Namespace.of("nolocation");
Review Comment:
I need to check, however, as I recall Hive is case-insensitive for database, table, and partition identifiers, typically converting them to lowercase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2023396642
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -445,7 +444,7 @@ public void testCreateTableCustomSortOrder() throws Exception {
@Test
public void testDatabaseAndNamespaceWithLocation() throws Exception {
- Namespace namespace1 = Namespace.of("noLocation");
+ Namespace namespace1 = Namespace.of("nolocation");
Review Comment:
btw, in my test `create database iCe;` directory is lower-cased:
````
aws s3 ls s3://xxx/warehouse/tablespace/external/hive/ice.db
PRE ice.db/
````
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2030391071
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java:
##########
@@ -303,10 +303,10 @@ public void testThriftExceptionConcurrentCommit() throws TException, Interrupted
@Test
public void testInvalidObjectException() {
- TableIdentifier badTi = TableIdentifier.of(DB_NAME, "`tbl`");
+ TableIdentifier badTi = TableIdentifier.of(DB_NAME, "tábl");
Review Comment:
Do we need to test changes before and after hive4?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#issuecomment-2781908061
I'm +1 for this approach, like how we've supported multiple versions of Flink and Spark. I'm suggesting to keep everything under `hive-metatore` directory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2020289548
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -445,7 +444,7 @@ public void testCreateTableCustomSortOrder() throws Exception {
@Test
public void testDatabaseAndNamespaceWithLocation() throws Exception {
- Namespace namespace1 = Namespace.of("noLocation");
+ Namespace namespace1 = Namespace.of("nolocation");
Review Comment:
For some reason, in Hive 4, the `org.apache.hadoop.hive.metastore.api.Database` below (`database1`) returns all-lowercase "nolocation.db" when `getLocationUri()` is called.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#issuecomment-2869268792
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2019739346
##########
hive4-metastore/src/test/java/org/apache/iceberg/hive/TestHiveMetastore.java:
##########
@@ -0,0 +1,252 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.hive;
+
+import static java.nio.file.Files.createTempDirectory;
+import static java.nio.file.attribute.PosixFilePermissions.asFileAttribute;
+import static java.nio.file.attribute.PosixFilePermissions.fromString;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.metastore.HMSHandler;
+import org.apache.hadoop.hive.metastore.HMSHandlerProxyFactory;
+import org.apache.hadoop.hive.metastore.IHMSHandler;
+import org.apache.hadoop.hive.metastore.TSetIpAddressProcessor;
+import org.apache.hadoop.hive.metastore.ThreadPool;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.hadoop.Util;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.thrift.TException;
+import org.apache.thrift.protocol.TBinaryProtocol;
+import org.apache.thrift.server.TServer;
+import org.apache.thrift.server.TThreadPoolServer;
+import org.apache.thrift.transport.TServerSocket;
+import org.apache.thrift.transport.TTransportFactory;
+
+public class TestHiveMetastore {
+
+ private static final String DEFAULT_DATABASE_NAME = "default";
+ private static final int DEFAULT_POOL_SIZE = 5;
+
+ // It's tricky to clear all static fields in an HMS instance in order to switch derby root dir.
+ // Therefore, we reuse the same derby root between tests and remove it after JVM exits.
+ private static final File HIVE_LOCAL_DIR;
+ private static final String DERBY_PATH;
+
+ static {
+ try {
+ HIVE_LOCAL_DIR =
+ createTempDirectory("hive", asFileAttribute(fromString("rwxrwxrwx"))).toFile();
+ DERBY_PATH = new File(HIVE_LOCAL_DIR, "metastore_db").getPath();
+ File derbyLogFile = new File(HIVE_LOCAL_DIR, "derby.log");
+ System.setProperty("derby.stream.error.file", derbyLogFile.getAbsolutePath());
+ setupMetastoreDB("jdbc:derby:" + DERBY_PATH + ";create=true");
+ Runtime.getRuntime()
+ .addShutdownHook(
+ new Thread(
+ () -> {
+ Path localDirPath = new Path(HIVE_LOCAL_DIR.getAbsolutePath());
+ FileSystem fs = Util.getFs(localDirPath, new Configuration());
+ String errMsg = "Failed to delete " + localDirPath;
+ try {
+ assertThat(fs.delete(localDirPath, true)).as(errMsg).isTrue();
+ } catch (IOException e) {
+ throw new RuntimeException(errMsg, e);
+ }
+ }));
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to setup local dir for hive metastore", e);
+ }
+ }
+
+ private HiveConf hiveConf;
+ private ExecutorService executorService;
+ private TServer server;
+ private HMSHandler baseHandler;
+ private HiveClientPool clientPool;
+
+ /**
+ * Starts a TestHiveMetastore with the default connection pool size (5) and the default HiveConf.
+ */
+ public void start() {
+ start(new HiveConf(new Configuration(), TestHiveMetastore.class), DEFAULT_POOL_SIZE);
+ }
+
+ /**
+ * Starts a TestHiveMetastore with the default connection pool size (5) with the provided
+ * HiveConf.
+ *
+ * @param conf The hive configuration to use
+ */
+ public void start(HiveConf conf) {
+ start(conf, DEFAULT_POOL_SIZE);
+ }
+
+ /**
+ * Starts a TestHiveMetastore with a provided connection pool size and HiveConf.
+ *
+ * @param conf The hive configuration to use
+ * @param poolSize The number of threads in the executor pool
+ */
+ public void start(HiveConf conf, int poolSize) {
+ try {
+ TServerSocket socket = new TServerSocket(0);
+ int port = socket.getServerSocket().getLocalPort();
+ initConf(conf, port);
+
+ this.hiveConf = conf;
+ this.server = newThriftServer(socket, poolSize, hiveConf);
+ this.executorService = Executors.newSingleThreadExecutor();
+ this.executorService.submit(() -> server.serve());
+
+ // in Hive3, setting this as a system prop ensures that it will be picked up whenever a new
+ // HiveConf is created
+ System.setProperty(
+ "hive.metastore.uris", hiveConf.getVar(HiveConf.getConfVars("hive.metastore.uris")));
+
+ this.clientPool = new HiveClientPool(1, hiveConf);
+ } catch (Exception e) {
+ throw new RuntimeException("Cannot start TestHiveMetastore", e);
+ }
+ }
+
+ public void stop() throws Exception {
+ reset();
+ if (clientPool != null) {
+ clientPool.close();
+ }
+ if (server != null) {
+ server.stop();
+ }
+ if (executorService != null) {
+ executorService.shutdown();
+ }
+ if (baseHandler != null) {
+ baseHandler.shutdown();
+ }
+ ThreadPool.shutdown();
+ }
+
+ public HiveConf hiveConf() {
+ return hiveConf;
+ }
+
+ public String getDatabasePath(String dbName) {
+ File dbDir = new File(HIVE_LOCAL_DIR, dbName + ".db");
+ return dbDir.getPath();
+ }
+
+ public void reset() throws Exception {
+ if (clientPool != null) {
+ for (String dbName : clientPool.run(client -> client.getAllDatabases())) {
+ for (String tblName : clientPool.run(client -> client.getAllTables(dbName))) {
+ clientPool.run(
+ client -> {
+ client.dropTable(dbName, tblName, true, true, true);
+ return null;
+ });
+ }
+
+ if (!DEFAULT_DATABASE_NAME.equals(dbName)) {
+ // Drop cascade, functions dropped by cascade
+ clientPool.run(
+ client -> {
+ client.dropDatabase(dbName, true, true, true);
+ return null;
+ });
+ }
+ }
+ }
+
+ Path warehouseRoot = new Path(HIVE_LOCAL_DIR.getAbsolutePath());
+ FileSystem fs = Util.getFs(warehouseRoot, hiveConf);
+ for (FileStatus fileStatus : fs.listStatus(warehouseRoot)) {
+ if (!fileStatus.getPath().getName().equals("derby.log")
+ && !fileStatus.getPath().getName().equals("metastore_db")) {
+ fs.delete(fileStatus.getPath(), true);
+ }
+ }
+ }
+
+ public Table getTable(String dbName, String tableName) throws TException, InterruptedException {
+ return clientPool.run(client -> client.getTable(dbName, tableName));
+ }
+
+ public Table getTable(TableIdentifier identifier) throws TException, InterruptedException {
+ return getTable(identifier.namespace().toString(), identifier.name());
+ }
+
+ private TServer newThriftServer(TServerSocket socket, int poolSize, HiveConf conf)
+ throws Exception {
+ HiveConf serverConf = new HiveConf(conf);
+ serverConf.set("javax.jdo.option.ConnectionURL", "jdbc:derby:" + DERBY_PATH + ";create=true");
+ baseHandler = new HMSHandler("new db based metaserver", serverConf);
+ IHMSHandler handler = HMSHandlerProxyFactory.getProxy(serverConf, baseHandler, false);
+
+ TThreadPoolServer.Args args =
+ new TThreadPoolServer.Args(socket)
+ .processor(new TSetIpAddressProcessor<>(handler))
+ .transportFactory(new TTransportFactory())
+ .protocolFactory(new TBinaryProtocol.Factory())
+ .minWorkerThreads(poolSize)
+ .maxWorkerThreads(poolSize);
+
+ return new TThreadPoolServer(args);
+ }
+
+ private void initConf(HiveConf conf, int port) {
+ conf.set("hive.metastore.uris", "thrift://localhost:" + port);
+ conf.set("hive.metastore.warehouse.dir", "file:" + HIVE_LOCAL_DIR.getAbsolutePath());
+ conf.set("hive.metastore.try.direct.sql", "false");
+ conf.set("hive.metastore.disallow.incompatible.col.type.changes", "false");
+ conf.set("iceberg.hive.client-pool-size", "2");
+ // Setting this to avoid thrift exception during running Iceberg tests outside Iceberg.
+ conf.set("hive.in.test", "false");
+ }
+
+ private static void setupMetastoreDB(String dbURL) throws SQLException, IOException {
+ try (Connection connection = DriverManager.getConnection(dbURL)) {
+ ScriptRunner scriptRunner = new ScriptRunner(connection, true, true);
+ try (InputStream inputStream =
+ TestHiveMetastore.class
+ .getClassLoader()
+ .getResourceAsStream("hive-schema-4.0.0.derby.sql");
Review Comment:
New schema needed for Hive 4.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2023308361
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -445,7 +444,7 @@ public void testCreateTableCustomSortOrder() throws Exception {
@Test
public void testDatabaseAndNamespaceWithLocation() throws Exception {
- Namespace namespace1 = Namespace.of("noLocation");
+ Namespace namespace1 = Namespace.of("nolocation");
Review Comment:
With `Namespace.of("noLocation")`, when `HiveCatalog::createNamespace` is called, the metastore client calls `createDatabase` with a `Database` containing name "noLocation" and locationUri ending with "noLocation.db", but when the client calls `getDatabase`, it gets back a `Database` containing name "nolocation" and locationUri ending with "nolocation.db". On disk, however, I can see that a directory "noLocation.db" has been created in the warehouse directory.
@pvary @deniskuzZ could you shed light on what changed in Hive 4 that causes this behavior? Is it anything we need to do something about on the Iceberg side?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2030387608
##########
build.gradle:
##########
@@ -763,6 +770,167 @@ project(':iceberg-hive-metastore') {
}
}
+project(':iceberg-hive3-metastore') {
Review Comment:
It might be better to put everything under `hive-metastore` directory.
```
hive-metastore/v2
hive-metastore/v3
hive-metastore/v4
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2032163482
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/CachedClientPool.java:
##########
@@ -132,7 +131,7 @@ public <R> R run(Action<R, IMetaStoreClient, TException> action, boolean retry)
static Key extractKey(String cacheKeys, Configuration conf) {
// generate key elements in a certain order, so that the Key instances are comparable
List<Object> elements = Lists.newArrayList();
- elements.add(conf.get(HiveConf.ConfVars.METASTOREURIS.varname, ""));
+ elements.add(conf.get("hive.metastore.uris", ""));
Review Comment:
I think it is not necessary and I don't see the benefit. Spark had to deal with this issue too, and they used the conf name or `HiveConf.getConfVars("...")` with the conf name if they needed a enum instance. They didn't go so far as to define a collection of constants for the conf names.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2023418469
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -445,7 +444,7 @@ public void testCreateTableCustomSortOrder() throws Exception {
@Test
public void testDatabaseAndNamespaceWithLocation() throws Exception {
- Namespace namespace1 = Namespace.of("noLocation");
+ Namespace namespace1 = Namespace.of("nolocation");
Review Comment:
difference is probably because Hive doesn't normally use `HiveCatalog#createNamespace`, only `HMS RestCatalog` does
````
database.setLocationUri(new Path(getExternalWarehouseLocation(), namespace.level(0)).toString() + ".db");
````
HMS (code hasn't changed from 2018)
````
private String dbDirFromDbName(Database db) throws MetaException {
return db.getName().toLowerCase() + DATABASE_WAREHOUSE_SUFFIX;
}
````
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#issuecomment-2779908426
@pvary I have rebased on main.
I see that this picked up https://github.com/apache/iceberg/commit/c661a71091e496393c743ddd879d9e1a0f2747b2 / https://github.com/apache/iceberg/pull/12637. In that change, a new test was added: `TestHiveCommitLocks::testMultipleAlterTableForNoLock`. This fails for Hive 3.1.3. I checked https://issues.apache.org/jira/browse/HIVE-26882 and see that it is fixed in 2.3.10 and 4.0.0-beta-1; it does not seem to be fixed in Hive 3 as there is no release after 3.1.3. Based on the date of the last commit in https://github.com/apache/hive/commits/rel/release-3.1.3/ and the date https://github.com/apache/hive/pull/3944 was merged, I conclude that HIVE-26882 is not available in 3.1.3.
Based on my understanding, it is not safe to set `engine.hive.lock-enabled` to false if HIVE-26882 is not available in the HMS version. @lirui-apache can you confirm? I have disabled `TestHiveCommitLocks::testMultipleAlterTableForNoLock` for Hive 3 to avoid the failure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2030390565
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java:
##########
@@ -303,10 +303,10 @@ public void testThriftExceptionConcurrentCommit() throws TException, Interrupted
@Test
public void testInvalidObjectException() {
- TableIdentifier badTi = TableIdentifier.of(DB_NAME, "`tbl`");
+ TableIdentifier badTi = TableIdentifier.of(DB_NAME, "tábl");
Review Comment:
Is this test still for hive2?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#issuecomment-2777665104
@pvary see https://github.com/apache/iceberg/pull/12721.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#issuecomment-2763196000
Note: TestHiveClientPool.java is the same for Hive 3 and 4 due to a change introduced in Hive 3 that is incompatible with Hive 2 (in fact only one test is changed). TestHiveMetastore.java is the same for Hive 2 and 3 and just different for Hive 4. We could deduplicate further but I didn't think it worthwhile. To keep things simple, if a file cannot be the same for all three Hive versions, I separate it out to three files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2020290971
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java:
##########
@@ -303,10 +303,10 @@ public void testThriftExceptionConcurrentCommit() throws TException, Interrupted
@Test
public void testInvalidObjectException() {
- TableIdentifier badTi = TableIdentifier.of(DB_NAME, "`tbl`");
+ TableIdentifier badTi = TableIdentifier.of(DB_NAME, "tábl");
Review Comment:
Before Hive 4, the only special character allowed is '/':
https://github.com/apache/hive/blob/branch-3.1/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java#L155
```
private static final char[] specialCharactersInTableNames = new char[] { '/' };
```
In Hive 4, all non-alphanumeric characters in a US keyboard, including backtick ('`'), are allowed special characters:
https://github.com/apache/hive/blob/master/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java#L175-L181
```
private static final char[] SPECIAL_CHARACTERS_IN_TABLE_NAMES = new char[] {
// standard
' ', '"', '%', '&', '\'', '(', ')', '*', '+', ',', '-', '.', '/', ':', ';', '<', '=', '>', '?', '[', ']',
'_', '|', '{', '}', '$', '^',
// non-standard
'!', '~', '#', '@', '`'
};
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2019738992
##########
hive3-metastore/src/test/java/org/apache/iceberg/hive/TestHiveMetastore.java:
##########
@@ -0,0 +1,285 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.hive;
+
+import static java.nio.file.Files.createTempDirectory;
+import static java.nio.file.attribute.PosixFilePermissions.asFileAttribute;
+import static java.nio.file.attribute.PosixFilePermissions.fromString;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.metastore.HiveMetaStore;
+import org.apache.hadoop.hive.metastore.IHMSHandler;
+import org.apache.hadoop.hive.metastore.RetryingHMSHandler;
+import org.apache.hadoop.hive.metastore.TSetIpAddressProcessor;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.common.DynConstructors;
+import org.apache.iceberg.common.DynMethods;
+import org.apache.iceberg.hadoop.Util;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.thrift.TException;
+import org.apache.thrift.protocol.TBinaryProtocol;
+import org.apache.thrift.server.TServer;
+import org.apache.thrift.server.TThreadPoolServer;
+import org.apache.thrift.transport.TServerSocket;
+import org.apache.thrift.transport.TTransportFactory;
+
+public class TestHiveMetastore {
+
+ private static final String DEFAULT_DATABASE_NAME = "default";
+ private static final int DEFAULT_POOL_SIZE = 5;
+
+ // create the metastore handlers based on whether we're working with Hive2 or Hive3 dependencies
+ // we need to do this because there is a breaking API change between Hive2 and Hive3
+ private static final DynConstructors.Ctor<HiveMetaStore.HMSHandler> HMS_HANDLER_CTOR =
+ DynConstructors.builder()
+ .impl(HiveMetaStore.HMSHandler.class, String.class, Configuration.class)
+ .impl(HiveMetaStore.HMSHandler.class, String.class, HiveConf.class)
+ .build();
+
+ private static final DynMethods.StaticMethod GET_BASE_HMS_HANDLER =
+ DynMethods.builder("getProxy")
+ .impl(RetryingHMSHandler.class, Configuration.class, IHMSHandler.class, boolean.class)
+ .impl(RetryingHMSHandler.class, HiveConf.class, IHMSHandler.class, boolean.class)
+ .buildStatic();
+
+ // Hive3 introduces background metastore tasks (MetastoreTaskThread) for performing various
+ // cleanup duties. These
+ // threads are scheduled and executed in a static thread pool
+ // (org.apache.hadoop.hive.metastore.ThreadPool).
+ // This thread pool is shut down normally as part of the JVM shutdown hook, but since we're
+ // creating and tearing down
+ // multiple metastore instances within the same JVM, we have to call this cleanup method manually,
+ // otherwise
+ // threads from our previous test suite will be stuck in the pool with stale config, and keep on
+ // being scheduled.
+ // This can lead to issues, e.g. accidental Persistence Manager closure by
+ // ScheduledQueryExecutionsMaintTask.
+ private static final DynMethods.StaticMethod METASTORE_THREADS_SHUTDOWN =
+ DynMethods.builder("shutdown")
+ .impl("org.apache.hadoop.hive.metastore.ThreadPool")
+ .orNoop()
+ .buildStatic();
+
+ // It's tricky to clear all static fields in an HMS instance in order to switch derby root dir.
+ // Therefore, we reuse the same derby root between tests and remove it after JVM exits.
+ private static final File HIVE_LOCAL_DIR;
+ private static final String DERBY_PATH;
+
+ static {
+ try {
+ HIVE_LOCAL_DIR =
+ createTempDirectory("hive", asFileAttribute(fromString("rwxrwxrwx"))).toFile();
+ DERBY_PATH = new File(HIVE_LOCAL_DIR, "metastore_db").getPath();
+ File derbyLogFile = new File(HIVE_LOCAL_DIR, "derby.log");
+ System.setProperty("derby.stream.error.file", derbyLogFile.getAbsolutePath());
+ setupMetastoreDB("jdbc:derby:" + DERBY_PATH + ";create=true");
+ Runtime.getRuntime()
+ .addShutdownHook(
+ new Thread(
+ () -> {
+ Path localDirPath = new Path(HIVE_LOCAL_DIR.getAbsolutePath());
+ FileSystem fs = Util.getFs(localDirPath, new Configuration());
+ String errMsg = "Failed to delete " + localDirPath;
+ try {
+ assertThat(fs.delete(localDirPath, true)).as(errMsg).isTrue();
+ } catch (IOException e) {
+ throw new RuntimeException(errMsg, e);
+ }
+ }));
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to setup local dir for hive metastore", e);
+ }
+ }
+
+ private HiveConf hiveConf;
+ private ExecutorService executorService;
+ private TServer server;
+ private HiveMetaStore.HMSHandler baseHandler;
+ private HiveClientPool clientPool;
+
+ /**
+ * Starts a TestHiveMetastore with the default connection pool size (5) and the default HiveConf.
+ */
+ public void start() {
+ start(new HiveConf(new Configuration(), TestHiveMetastore.class), DEFAULT_POOL_SIZE);
+ }
+
+ /**
+ * Starts a TestHiveMetastore with the default connection pool size (5) with the provided
+ * HiveConf.
+ *
+ * @param conf The hive configuration to use
+ */
+ public void start(HiveConf conf) {
+ start(conf, DEFAULT_POOL_SIZE);
+ }
+
+ /**
+ * Starts a TestHiveMetastore with a provided connection pool size and HiveConf.
+ *
+ * @param conf The hive configuration to use
+ * @param poolSize The number of threads in the executor pool
+ */
+ public void start(HiveConf conf, int poolSize) {
+ try {
+ TServerSocket socket = new TServerSocket(0);
+ int port = socket.getServerSocket().getLocalPort();
+ initConf(conf, port);
+
+ this.hiveConf = conf;
+ this.server = newThriftServer(socket, poolSize, hiveConf);
+ this.executorService = Executors.newSingleThreadExecutor();
+ this.executorService.submit(() -> server.serve());
+
+ // in Hive3, setting this as a system prop ensures that it will be picked up whenever a new
+ // HiveConf is created
+ System.setProperty(
+ "hive.metastore.uris", hiveConf.getVar(HiveConf.getConfVars("hive.metastore.uris")));
+
+ this.clientPool = new HiveClientPool(1, hiveConf);
+ } catch (Exception e) {
+ throw new RuntimeException("Cannot start TestHiveMetastore", e);
+ }
+ }
+
+ public void stop() throws Exception {
+ reset();
+ if (clientPool != null) {
+ clientPool.close();
+ }
+ if (server != null) {
+ server.stop();
+ }
+ if (executorService != null) {
+ executorService.shutdown();
+ }
+ if (baseHandler != null) {
+ baseHandler.shutdown();
+ }
+ METASTORE_THREADS_SHUTDOWN.invoke();
+ }
+
+ public HiveConf hiveConf() {
+ return hiveConf;
+ }
+
+ public String getDatabasePath(String dbName) {
+ File dbDir = new File(HIVE_LOCAL_DIR, dbName + ".db");
+ return dbDir.getPath();
+ }
+
+ public void reset() throws Exception {
+ if (clientPool != null) {
+ for (String dbName : clientPool.run(client -> client.getAllDatabases())) {
+ for (String tblName : clientPool.run(client -> client.getAllTables(dbName))) {
+ clientPool.run(
+ client -> {
+ client.dropTable(dbName, tblName, true, true, true);
+ return null;
+ });
+ }
+
+ if (!DEFAULT_DATABASE_NAME.equals(dbName)) {
+ // Drop cascade, functions dropped by cascade
+ clientPool.run(
+ client -> {
+ client.dropDatabase(dbName, true, true, true);
+ return null;
+ });
+ }
+ }
+ }
+
+ Path warehouseRoot = new Path(HIVE_LOCAL_DIR.getAbsolutePath());
+ FileSystem fs = Util.getFs(warehouseRoot, hiveConf);
+ for (FileStatus fileStatus : fs.listStatus(warehouseRoot)) {
+ if (!fileStatus.getPath().getName().equals("derby.log")
+ && !fileStatus.getPath().getName().equals("metastore_db")) {
+ fs.delete(fileStatus.getPath(), true);
+ }
+ }
+ }
+
+ public Table getTable(String dbName, String tableName) throws TException, InterruptedException {
+ return clientPool.run(client -> client.getTable(dbName, tableName));
+ }
+
+ public Table getTable(TableIdentifier identifier) throws TException, InterruptedException {
+ return getTable(identifier.namespace().toString(), identifier.name());
+ }
+
+ private TServer newThriftServer(TServerSocket socket, int poolSize, HiveConf conf)
+ throws Exception {
+ HiveConf serverConf = new HiveConf(conf);
+ serverConf.set("javax.jdo.option.ConnectionURL", "jdbc:derby:" + DERBY_PATH + ";create=true");
+ baseHandler = HMS_HANDLER_CTOR.newInstance("new db based metaserver", serverConf);
+ IHMSHandler handler = GET_BASE_HMS_HANDLER.invoke(serverConf, baseHandler, false);
+
+ TThreadPoolServer.Args args =
+ new TThreadPoolServer.Args(socket)
+ .processor(new TSetIpAddressProcessor<>(handler))
+ .transportFactory(new TTransportFactory())
+ .protocolFactory(new TBinaryProtocol.Factory())
+ .minWorkerThreads(poolSize)
+ .maxWorkerThreads(poolSize);
+
+ return new TThreadPoolServer(args);
+ }
+
+ private void initConf(HiveConf conf, int port) {
+ conf.set("hive.metastore.uris", "thrift://localhost:" + port);
+ conf.set("hive.metastore.warehouse.dir", "file:" + HIVE_LOCAL_DIR.getAbsolutePath());
+ conf.set("hive.metastore.try.direct.sql", "false");
+ conf.set("hive.metastore.disallow.incompatible.col.type.changes", "false");
+ conf.set("iceberg.hive.client-pool-size", "2");
+ // Setting this to avoid thrift exception during running Iceberg tests outside Iceberg.
+ conf.set("hive.in.test", "false");
+ }
+
+ private static void setupMetastoreDB(String dbURL) throws SQLException, IOException {
+ try (Connection connection = DriverManager.getConnection(dbURL)) {
+ ScriptRunner scriptRunner = new ScriptRunner(connection, true, true);
+ try (InputStream inputStream =
+ TestHiveMetastore.class
+ .getClassLoader()
+ .getResourceAsStream("hive-schema-3.1.0.derby.sql");
Review Comment:
This can be used for both Hive 2 and Hive 3.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2023418469
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -445,7 +444,7 @@ public void testCreateTableCustomSortOrder() throws Exception {
@Test
public void testDatabaseAndNamespaceWithLocation() throws Exception {
- Namespace namespace1 = Namespace.of("noLocation");
+ Namespace namespace1 = Namespace.of("nolocation");
Review Comment:
difference is probably because Hive doesn't normally use `HiveCatalog#createNamespace` only `HMS RestCatalog` does
````
database.setLocationUri(new Path(getExternalWarehouseLocation(), namespace.level(0)).toString() + ".db");
````
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -445,7 +444,7 @@ public void testCreateTableCustomSortOrder() throws Exception {
@Test
public void testDatabaseAndNamespaceWithLocation() throws Exception {
- Namespace namespace1 = Namespace.of("noLocation");
+ Namespace namespace1 = Namespace.of("nolocation");
Review Comment:
difference is probably because Hive doesn't normally use `HiveCatalog#createNamespace`, only `HMS RestCatalog` does
````
database.setLocationUri(new Path(getExternalWarehouseLocation(), namespace.level(0)).toString() + ".db");
````
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2023418469
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -445,7 +444,7 @@ public void testCreateTableCustomSortOrder() throws Exception {
@Test
public void testDatabaseAndNamespaceWithLocation() throws Exception {
- Namespace namespace1 = Namespace.of("noLocation");
+ Namespace namespace1 = Namespace.of("nolocation");
Review Comment:
difference is probably because Hive doesn't normally use `HiveCatalog#createNamespace` only `HMS RestCatalog`
````
database.setLocationUri(new Path(getExternalWarehouseLocation(), namespace.level(0)).toString() + ".db");
````
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2030387861
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/CachedClientPool.java:
##########
@@ -132,7 +131,7 @@ public <R> R run(Action<R, IMetaStoreClient, TException> action, boolean retry)
static Key extractKey(String cacheKeys, Configuration conf) {
// generate key elements in a certain order, so that the Key instances are comparable
List<Object> elements = Lists.newArrayList();
- elements.add(conf.get(HiveConf.ConfVars.METASTOREURIS.varname, ""));
+ elements.add(conf.get("hive.metastore.uris", ""));
Review Comment:
It might be better to define this constant in an Iceberg util class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "lirui-apache (via GitHub)" <gi...@apache.org>.
lirui-apache commented on PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#issuecomment-2781896981
@wypoon The no-lock feature needs both HIVE-26882 and HIVE-28121 to work, which are not available in Hive 3.1.3. So +1 to skipping the test for Hive 3.x.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2019738809
##########
hive3-metastore/src/test/java/org/apache/iceberg/hive/TestHiveMetastore.java:
##########
@@ -0,0 +1,285 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.hive;
+
+import static java.nio.file.Files.createTempDirectory;
+import static java.nio.file.attribute.PosixFilePermissions.asFileAttribute;
+import static java.nio.file.attribute.PosixFilePermissions.fromString;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.metastore.HiveMetaStore;
+import org.apache.hadoop.hive.metastore.IHMSHandler;
+import org.apache.hadoop.hive.metastore.RetryingHMSHandler;
+import org.apache.hadoop.hive.metastore.TSetIpAddressProcessor;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.common.DynConstructors;
+import org.apache.iceberg.common.DynMethods;
+import org.apache.iceberg.hadoop.Util;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.thrift.TException;
+import org.apache.thrift.protocol.TBinaryProtocol;
+import org.apache.thrift.server.TServer;
+import org.apache.thrift.server.TThreadPoolServer;
+import org.apache.thrift.transport.TServerSocket;
+import org.apache.thrift.transport.TTransportFactory;
+
+public class TestHiveMetastore {
+
+ private static final String DEFAULT_DATABASE_NAME = "default";
+ private static final int DEFAULT_POOL_SIZE = 5;
+
+ // create the metastore handlers based on whether we're working with Hive2 or Hive3 dependencies
+ // we need to do this because there is a breaking API change between Hive2 and Hive3
Review Comment:
This file is exactly the same as TestHiveMetastore.java in hive2-metastore.
Therefore the `DynConstructors` and `DynMethods` machinery is kept/used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2019739243
##########
hive4-metastore/src/test/java/org/apache/iceberg/hive/TestHiveMetastore.java:
##########
@@ -0,0 +1,252 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.hive;
+
+import static java.nio.file.Files.createTempDirectory;
+import static java.nio.file.attribute.PosixFilePermissions.asFileAttribute;
+import static java.nio.file.attribute.PosixFilePermissions.fromString;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.metastore.HMSHandler;
+import org.apache.hadoop.hive.metastore.HMSHandlerProxyFactory;
+import org.apache.hadoop.hive.metastore.IHMSHandler;
+import org.apache.hadoop.hive.metastore.TSetIpAddressProcessor;
+import org.apache.hadoop.hive.metastore.ThreadPool;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.hadoop.Util;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.thrift.TException;
+import org.apache.thrift.protocol.TBinaryProtocol;
+import org.apache.thrift.server.TServer;
+import org.apache.thrift.server.TThreadPoolServer;
+import org.apache.thrift.transport.TServerSocket;
+import org.apache.thrift.transport.TTransportFactory;
+
+public class TestHiveMetastore {
+
+ private static final String DEFAULT_DATABASE_NAME = "default";
+ private static final int DEFAULT_POOL_SIZE = 5;
+
+ // It's tricky to clear all static fields in an HMS instance in order to switch derby root dir.
+ // Therefore, we reuse the same derby root between tests and remove it after JVM exits.
+ private static final File HIVE_LOCAL_DIR;
+ private static final String DERBY_PATH;
+
+ static {
+ try {
+ HIVE_LOCAL_DIR =
+ createTempDirectory("hive", asFileAttribute(fromString("rwxrwxrwx"))).toFile();
+ DERBY_PATH = new File(HIVE_LOCAL_DIR, "metastore_db").getPath();
+ File derbyLogFile = new File(HIVE_LOCAL_DIR, "derby.log");
+ System.setProperty("derby.stream.error.file", derbyLogFile.getAbsolutePath());
+ setupMetastoreDB("jdbc:derby:" + DERBY_PATH + ";create=true");
+ Runtime.getRuntime()
+ .addShutdownHook(
+ new Thread(
+ () -> {
+ Path localDirPath = new Path(HIVE_LOCAL_DIR.getAbsolutePath());
+ FileSystem fs = Util.getFs(localDirPath, new Configuration());
+ String errMsg = "Failed to delete " + localDirPath;
+ try {
+ assertThat(fs.delete(localDirPath, true)).as(errMsg).isTrue();
+ } catch (IOException e) {
+ throw new RuntimeException(errMsg, e);
+ }
+ }));
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to setup local dir for hive metastore", e);
+ }
+ }
+
+ private HiveConf hiveConf;
+ private ExecutorService executorService;
+ private TServer server;
+ private HMSHandler baseHandler;
+ private HiveClientPool clientPool;
+
+ /**
+ * Starts a TestHiveMetastore with the default connection pool size (5) and the default HiveConf.
+ */
+ public void start() {
+ start(new HiveConf(new Configuration(), TestHiveMetastore.class), DEFAULT_POOL_SIZE);
+ }
+
+ /**
+ * Starts a TestHiveMetastore with the default connection pool size (5) with the provided
+ * HiveConf.
+ *
+ * @param conf The hive configuration to use
+ */
+ public void start(HiveConf conf) {
+ start(conf, DEFAULT_POOL_SIZE);
+ }
+
+ /**
+ * Starts a TestHiveMetastore with a provided connection pool size and HiveConf.
+ *
+ * @param conf The hive configuration to use
+ * @param poolSize The number of threads in the executor pool
+ */
+ public void start(HiveConf conf, int poolSize) {
+ try {
+ TServerSocket socket = new TServerSocket(0);
+ int port = socket.getServerSocket().getLocalPort();
+ initConf(conf, port);
+
+ this.hiveConf = conf;
+ this.server = newThriftServer(socket, poolSize, hiveConf);
+ this.executorService = Executors.newSingleThreadExecutor();
+ this.executorService.submit(() -> server.serve());
+
+ // in Hive3, setting this as a system prop ensures that it will be picked up whenever a new
+ // HiveConf is created
+ System.setProperty(
+ "hive.metastore.uris", hiveConf.getVar(HiveConf.getConfVars("hive.metastore.uris")));
+
+ this.clientPool = new HiveClientPool(1, hiveConf);
+ } catch (Exception e) {
+ throw new RuntimeException("Cannot start TestHiveMetastore", e);
+ }
+ }
+
+ public void stop() throws Exception {
+ reset();
+ if (clientPool != null) {
+ clientPool.close();
+ }
+ if (server != null) {
+ server.stop();
+ }
+ if (executorService != null) {
+ executorService.shutdown();
+ }
+ if (baseHandler != null) {
+ baseHandler.shutdown();
+ }
+ ThreadPool.shutdown();
+ }
+
+ public HiveConf hiveConf() {
+ return hiveConf;
+ }
+
+ public String getDatabasePath(String dbName) {
+ File dbDir = new File(HIVE_LOCAL_DIR, dbName + ".db");
+ return dbDir.getPath();
+ }
+
+ public void reset() throws Exception {
+ if (clientPool != null) {
+ for (String dbName : clientPool.run(client -> client.getAllDatabases())) {
+ for (String tblName : clientPool.run(client -> client.getAllTables(dbName))) {
+ clientPool.run(
+ client -> {
+ client.dropTable(dbName, tblName, true, true, true);
+ return null;
+ });
+ }
+
+ if (!DEFAULT_DATABASE_NAME.equals(dbName)) {
+ // Drop cascade, functions dropped by cascade
+ clientPool.run(
+ client -> {
+ client.dropDatabase(dbName, true, true, true);
+ return null;
+ });
+ }
+ }
+ }
+
+ Path warehouseRoot = new Path(HIVE_LOCAL_DIR.getAbsolutePath());
+ FileSystem fs = Util.getFs(warehouseRoot, hiveConf);
+ for (FileStatus fileStatus : fs.listStatus(warehouseRoot)) {
+ if (!fileStatus.getPath().getName().equals("derby.log")
+ && !fileStatus.getPath().getName().equals("metastore_db")) {
+ fs.delete(fileStatus.getPath(), true);
+ }
+ }
+ }
+
+ public Table getTable(String dbName, String tableName) throws TException, InterruptedException {
+ return clientPool.run(client -> client.getTable(dbName, tableName));
+ }
+
+ public Table getTable(TableIdentifier identifier) throws TException, InterruptedException {
+ return getTable(identifier.namespace().toString(), identifier.name());
+ }
+
+ private TServer newThriftServer(TServerSocket socket, int poolSize, HiveConf conf)
+ throws Exception {
+ HiveConf serverConf = new HiveConf(conf);
+ serverConf.set("javax.jdo.option.ConnectionURL", "jdbc:derby:" + DERBY_PATH + ";create=true");
+ baseHandler = new HMSHandler("new db based metaserver", serverConf);
+ IHMSHandler handler = HMSHandlerProxyFactory.getProxy(serverConf, baseHandler, false);
Review Comment:
Since this file is only for Hive 4, we don't need to use the `DynConstructors` and `DynMethods` machinery. We simply call the relevant constructor and static method directly!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2023418469
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -445,7 +444,7 @@ public void testCreateTableCustomSortOrder() throws Exception {
@Test
public void testDatabaseAndNamespaceWithLocation() throws Exception {
- Namespace namespace1 = Namespace.of("noLocation");
+ Namespace namespace1 = Namespace.of("nolocation");
Review Comment:
difference is probably because Hive doesn't normally use `HiveCatalog#createNamespace`, only `HMS RestCatalog` does
````
database.setLocationUri(new Path(getExternalWarehouseLocation(), namespace.level(0)).toString() + ".db");
````
HMS
````
private String dbDirFromDbName(Database db) throws MetaException {
return db.getName().toLowerCase() + DATABASE_WAREHOUSE_SUFFIX;
}
````
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#issuecomment-2888655066
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #12681: Build and test hive-metastore with Hive 3 and Hive 4
URL: https://github.com/apache/iceberg/pull/12681
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on code in PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#discussion_r2032146553
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java:
##########
@@ -303,10 +303,10 @@ public void testThriftExceptionConcurrentCommit() throws TException, Interrupted
@Test
public void testInvalidObjectException() {
- TableIdentifier badTi = TableIdentifier.of(DB_NAME, "`tbl`");
+ TableIdentifier badTi = TableIdentifier.of(DB_NAME, "tábl");
Review Comment:
No, we do not. The purpose of the test is simply to test exception handling when the table name is not accepted by Hive. For this purpose, any name not accepted by Hive would do. Previously, a name containing the backtick character ('`') fit the bill, but now we need to use a different kind of name; that's all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#issuecomment-2784931578
> If you mean keep versioned directories under hive-metastore, but do not duplicate code unless necessary
Yes, this is what I mean.
If you don't want to move the existing `hive-metastore` module. How about adding an `hive-metastore-tests` directory and put `v2`, `v3` and `v4` under it? This directory can be later merged into `hive-metastore` when we deprecate hive2 and hive3 connectors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Build and test hive-metastore with Hive 3 and Hive 4 [iceberg]
Posted by "wypoon (via GitHub)" <gi...@apache.org>.
wypoon commented on PR #12681:
URL: https://github.com/apache/iceberg/pull/12681#issuecomment-2777659880
@pvary I replied on the dev list.
> I really would like to see HiveCatalog testing against Hive4, but Spark has constraints around testing newer Hive versions which we need to solve, or wait until all of the supported Spark version will have an updated Hive.
>
I think testing Flink and Spark with a Hive 3/Hive 4 metastore should be a separate PR. This is the first step. I also think this step makes it possible for the Hive project to drop its fork of this module from the Hive repo.
I also do not think we should wait until all supported Spark versions support Hive 4, for this is never going to happen for Spark 3.4 or even Spark 3.5 (which is a LTS version and which I expect Iceberg to support for a while).
> OTOH, if possible, I would like to see only a single TestHiveMetastore class. Could we do some changes internally to the class, which based on the classes available on the classpath, or some configuration would use the correct Hive versions (maybe with DynMethods)?
The problem is with the HMSHandler class. Yes, I can use `DynConstructors` to construct the `HMSHandler` using the classname based on the Hive version, but `TestHiveMetastore` holds an instance of `HMSHandler` in a field, `baseHandler`, and in `TestHiveMetastore::stop()`, it calls `baseHandler.shutdown()`. We need to declare this field, but the type of the field is `org.apache.hadoop.hive.metastore.HMSHandler` in Hive 4 and `org.apache.hadoop.hive.metastore.HiveMetaStore.HMSHandler` in Hive 2 and 3. I don't see how to declare that field in a way independent of the Hive version. The interface that `HMSHandler` implements, `IHMSHandler`, does not have a `shutdown()` method. I need to call that method on `HMSHandler`, not `IHMSHandler`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org