ajantha-bhat commented on code in PR #225:
URL: https://github.com/apache/polaris-tools/pull/225#discussion_r3226166280
##########
iceberg-catalog-migrator/api/src/main/java/org/apache/polaris/iceberg/catalog/migrator/api/CatalogMigrator.java:
##########
@@ -211,11 +220,42 @@ protected void
getAllNamespacesFromSourceCatalog(Namespace namespace, Set<Namesp
private boolean registerTableToTargetCatalog(TableIdentifier
tableIdentifier) {
try {
createNamespacesIfNotExistOnTargetCatalog(tableIdentifier.namespace());
- // register the table to the target catalog
- TableOperations ops = ((BaseTable)
sourceCatalog().loadTable(tableIdentifier)).operations();
- targetCatalog().registerTable(tableIdentifier,
ops.current().metadataFileLocation());
- LOG.info("Successfully registered the table {}", tableIdentifier);
- return true;
+
+ TableOperations sourceOps =
+ ((BaseTable)
sourceCatalog().loadTable(tableIdentifier)).operations();
+ String sourceMetadataLocation =
sourceOps.current().metadataFileLocation();
+
+ try {
+ targetCatalog().registerTable(tableIdentifier, sourceMetadataLocation);
+ LOG.info("Successfully registered the table {}", tableIdentifier);
+ return true;
+ } catch (AlreadyExistsException ex) {
+ if (overwrite()) {
+ TableOperations targetOps =
+ ((BaseTable)
targetCatalog().loadTable(tableIdentifier)).operations();
+ String targetMetadataLocation =
targetOps.current().metadataFileLocation();
+
+ if (sourceMetadataLocation.equals(targetMetadataLocation)) {
+ LOG.info(
+ "Table {} is already in sync with the target catalog. Skipping
re-registration.",
+ tableIdentifier);
+ return true;
+ }
+
+ LOG.info(
+ "Table {} metadata location has changed. Dropping and
re-registering it in the target catalog as overwrite is enabled.",
+ tableIdentifier);
+ targetCatalog().dropTable(tableIdentifier, false);
+ targetCatalog().registerTable(tableIdentifier,
sourceMetadataLocation);
Review Comment:
We don't have to handle like this.
Iceberg provides an API with the force register option already.
Refer this:
https://github.com/apache/iceberg/blob/3d682a3b65838aba29f16141b45633a6cbe51178/api/src/main/java/org/apache/iceberg/catalog/Catalog.java#L362
##########
iceberg-catalog-migrator/docs/examples.md:
##########
@@ -120,3 +120,14 @@ java -jar iceberg-catalog-migrator-cli-0.0.1.jar migrate \
## Tips
1. Before migrating tables to Polaris, make sure the catalog is configured to
the `base-location` same as source catalog `warehouse` location during catalog
creation.
+2. If you have already registered tables and the source catalog metadata has
changed (e.g., after table maintenance or new commits), you can use the
`--overwrite`
Review Comment:
This implies that user is using the same table from multiple catalogs. Which
is not recommended.
See here about the warning:
https://github.com/apache/polaris-tools/tree/main/iceberg-catalog-migrator#readme
##########
iceberg-catalog-migrator/build.gradle.kts:
##########
@@ -76,6 +76,9 @@ tasks.named<RatTask>("rat").configure {
// Rat can't scan binary images
excludes.add("**/*.png")
+
+ // Misc build artifacts
+ excludes.add(".java-version")
Review Comment:
unrelated changes to this PR?
##########
iceberg-catalog-migrator/.gitignore:
##########
@@ -17,6 +17,69 @@
# under the License.
#
+# Ignore Gradle GUI config
+gradle-app.setting
+
# Ignore Gradle wrapper jar file
gradle/wrapper/gradle-wrapper.jar
gradle/wrapper/gradle-wrapper-*.sha256
+
+# Avoid ignore Gradle wrappper properties
Review Comment:
Unrelated changes to this PR?
--
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]
