electrum commented on a change in pull request #240: Add HiveCatalog
implementation
URL: https://github.com/apache/incubator-iceberg/pull/240#discussion_r298791652
##########
File path: hive/src/main/java/org/apache/iceberg/hive/HiveCatalog.java
##########
@@ -0,0 +1,119 @@
+package org.apache.iceberg.hive;
+
+import java.io.Closeable;
+import java.util.Map;
+import com.google.common.base.Preconditions;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.api.NoSuchObjectException;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.iceberg.BaseMetastoreCatalog;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.TableOperations;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.thrift.TException;
+
+public class HiveCatalog extends BaseMetastoreCatalog implements Closeable {
+
+ private final HiveClientPool clients;
+
+ public HiveCatalog(Configuration conf) {
+ super(conf);
+ this.clients = new HiveClientPool(2, conf);
+ }
+
+ @Override
+ public org.apache.iceberg.Table createTable(
+ TableIdentifier identifier, Schema schema, PartitionSpec spec, String
location, Map<String, String> properties) {
+ Preconditions.checkArgument(identifier.namespace().levels().length == 1,
+ "Missing database in table identifier: %s", identifier);
+ return super.createTable(identifier, schema, spec, location, properties);
+ }
+
+ @Override
+ public org.apache.iceberg.Table loadTable(TableIdentifier identifier) {
+ Preconditions.checkArgument(identifier.namespace().levels().length == 1,
+ "Missing database in table identifier: %s", identifier);
+ return super.loadTable(identifier);
+ }
+
+ @Override
+ public boolean dropTable(TableIdentifier identifier) {
+ Preconditions.checkArgument(identifier.namespace().levels().length == 1,
+ "Missing database in table identifier: %s", identifier);
+ String database = identifier.namespace().level(0);
+
+ try {
+ clients.run(client -> {
+ client.dropTable(database, identifier.name());
Review comment:
My point was that this is confusing to readers, since we are asking the
metastore to delete the data, but we know it won’t be deleted since it’s an
external table.
And because the delete flag is set, existing Iceberg tables from before this
change may have the data deleted, since they are not external tables. (I
haven’t checked how metastore treats custom table types. Maybe same as
external, in which case this is ok.)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
