singhpk234 commented on code in PR #15010:
URL: https://github.com/apache/iceberg/pull/15010#discussion_r2688495137
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java:
##########
@@ -911,4 +911,27 @@ public View create() {
return super.create();
}
}
+
+ @Override
+ public org.apache.iceberg.Table registerTable(
+ TableIdentifier identifier, String metadataFileLocation) {
+ Preconditions.checkArgument(
+ identifier != null && isValidIdentifier(identifier), "Invalid
identifier: %s", identifier);
+ Preconditions.checkArgument(
+ metadataFileLocation != null && !metadataFileLocation.isEmpty(),
+ "Cannot register an empty metadata file location as a table");
+
+ // throw an exception in case the table identifier already exists as a
table/view
+ if (tableExists(identifier)) {
+ throw new org.apache.iceberg.exceptions.AlreadyExistsException(
+ "Table already exists: %s", identifier);
+ }
+
+ if (viewExists(identifier)) {
+ throw new org.apache.iceberg.exceptions.AlreadyExistsException(
+ "View with same name already exists: %s", identifier);
+ }
Review Comment:
Got you that make sense ! i believe we use the table and internal metadata
in hms if it a iceberg table or view then
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java:
##########
@@ -896,19 +890,48 @@ private TableAwareViewBuilder(TableIdentifier identifier)
{
@Override
public View createOrReplace() {
if (tableExists(identifier)) {
- throw new org.apache.iceberg.exceptions.AlreadyExistsException(
- "Table with same name already exists: %s", identifier);
+ throw new AlreadyExistsException("Table with same name already exists:
%s", identifier);
}
return super.createOrReplace();
}
@Override
public View create() {
if (tableExists(identifier)) {
- throw new org.apache.iceberg.exceptions.AlreadyExistsException(
- "Table with same name already exists: %s", identifier);
+ throw new AlreadyExistsException("Table with same name already exists:
%s", identifier);
}
return super.create();
}
}
+
+ /**
+ * Register a table with the catalog if it does not exist. This is
overridden in order to add view
+ * existence detection before registering a table.
+ *
+ * @param identifier a table identifier
+ * @param metadataFileLocation the location of a metadata file
+ * @return a Table instance
+ * @throws AlreadyExistsException if a table or view with the same
identifier already exists in
+ * the catalog.
+ */
+ @Override
+ public org.apache.iceberg.Table registerTable(
+ TableIdentifier identifier, String metadataFileLocation) {
+ Preconditions.checkArgument(
+ identifier != null && isValidIdentifier(identifier), "Invalid
identifier: %s", identifier);
+ Preconditions.checkArgument(
+ metadataFileLocation != null && !metadataFileLocation.isEmpty(),
+ "Cannot register an empty metadata file location as a table");
+
+ // throw an exception in case the table identifier already exists as a
table/view
+ if (tableExists(identifier)) {
+ throw new AlreadyExistsException("Table already exists: %s", identifier);
+ }
Review Comment:
i believe we call the tableExists call already in the `super.registerTable`
which has tableExists check do we still need this call here ?
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java#L82
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
