rajarshisarkar commented on a change in pull request #4100:
URL: https://github.com/apache/iceberg/pull/4100#discussion_r807908123
##########
File path:
aws/src/main/java/org/apache/iceberg/aws/dynamodb/DynamoDbCatalog.java
##########
@@ -440,6 +443,42 @@ public void close() throws IOException {
closeableGroup.close();
}
+ @Override
+ public Table registerTable(TableIdentifier identifier, String
metadataFileLocation) {
+ Preconditions.checkArgument(isValidIdentifier(identifier),
+ "Cannot register: invalid table identifier " + identifier);
+ Preconditions.checkArgument(metadataFileLocation != null &&
!metadataFileLocation.isEmpty(),
+ "Cannot register: empty metadata file location as a table");
+ if (!namespaceExists(identifier.namespace())) {
+ throw new NoSuchNamespaceException("Cannot register: namespace %s not
found", identifier.namespace());
+ }
+
+ Map<String, AttributeValue> values = tablePrimaryKey(identifier);
+ values.put(
+ toPropertyCol(BaseMetastoreTableOperations.TABLE_TYPE_PROP),
+ AttributeValue.builder()
+
.s(BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE.toUpperCase(Locale.ENGLISH))
+ .build());
+ values.put(
+ toPropertyCol(BaseMetastoreTableOperations.METADATA_LOCATION_PROP),
+ AttributeValue.builder()
+ .s(metadataFileLocation)
+ .build());
+ DynamoDbCatalog.setNewCatalogEntryMetadata(values);
+
+ try {
+ dynamo.putItem(PutItemRequest.builder()
+ .tableName(awsProperties.dynamoDbTableName())
+ .item(values)
+ .conditionExpression("attribute_not_exists(" + COL_VERSION + ")")
+ .build());
+ } catch (ConditionalCheckFailedException e) {
+ throw new AlreadyExistsException(e, "Cannot register: table %s already
exists", identifier);
+ }
Review comment:
Should we have a chained catch for `DynamoDbException` (Base class for
all service exceptions)? `putItems` seems to throw many other exceptions apart
from `ConditionalCheckFailedException`.
##########
File path:
aws/src/main/java/org/apache/iceberg/aws/dynamodb/DynamoDbCatalog.java
##########
@@ -440,6 +443,42 @@ public void close() throws IOException {
closeableGroup.close();
}
+ @Override
+ public Table registerTable(TableIdentifier identifier, String
metadataFileLocation) {
+ Preconditions.checkArgument(isValidIdentifier(identifier),
+ "Cannot register: invalid table identifier " + identifier);
Review comment:
`isValidIdentifier` for DynamoDB always returns true. Is this something
we can implement (maybe in a separate PR)?
##########
File path:
aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java
##########
@@ -295,6 +302,77 @@ public void testDropNamespace() {
Assert.assertFalse("namespace must not exist", response.hasItem());
}
+ @Test
+ public void testRegisterTable() {
+ Namespace namespace = Namespace.of(genRandomName());
+ catalog.createNamespace(namespace);
+ TableIdentifier tableIdentifier = TableIdentifier.of(namespace,
genRandomName());
+ catalog.createTable(tableIdentifier, SCHEMA);
+ Table table = catalog.loadTable(tableIdentifier);
+ DataFile dataFile = DataFiles.builder(PartitionSpec.unpartitioned())
+ .withPath("/path/to/data-a.parquet")
+ .withFileSizeInBytes(10)
+ .withRecordCount(1)
+ .build();
+ table.newAppend().appendFile(dataFile).commit();
+ table.refresh();
+ long v1SnapshotId = table.currentSnapshot().snapshotId();
+ String v1MetadataLocation = ((BaseTable)
table).operations().current().metadataFileLocation();
+ table.newDelete().deleteFile(dataFile).commit();
+ table.refresh();
Review comment:
I think we can avoid the refresh calls.
--
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]
