Skip to content

Commit 2e03e7f

Browse files
yabolayabola
committed
test
1 parent 5951ece commit 2e03e7f

File tree

23 files changed

+153
-44
lines changed

23 files changed

+153
-44
lines changed

‎.palantir/revapi.yml

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
versionOverrides:
22
org.apache.iceberg:iceberg-api:apache-iceberg-0.14.0: "0.14.0"
3+
org.apache.iceberg:iceberg-api:release-base-0.13.0: "111"
34
acceptedBreaks:
45
release-base-0.13.0:
56
org.apache.iceberg:iceberg-api:
@@ -47,10 +48,12 @@ acceptedBreaks:
4748
justification: "Allow adding a new method to the interface - old method is deprecated"
4849
- code: "java.method.addedToInterface"
4950
new: "method long org.apache.iceberg.actions.ExpireSnapshots.Result::deletedEqualityDeleteFilesCount()"
50-
justification: "Interface is backward compatible, very unlikely anyone implements this Result bean interface"
51+
justification: "Interface is backward compatible, very unlikely anyone implements\
52+
\ this Result bean interface"
5153
- code: "java.method.addedToInterface"
5254
new: "method long org.apache.iceberg.actions.ExpireSnapshots.Result::deletedPositionDeleteFilesCount()"
53-
justification: "Interface is backward compatible, very unlikely anyone implements this Result bean interface"
55+
justification: "Interface is backward compatible, very unlikely anyone implements\
56+
\ this Result bean interface"
5457
- code: "java.method.addedToInterface"
5558
new: "method org.apache.iceberg.ExpireSnapshots org.apache.iceberg.ExpireSnapshots::planWith(java.util.concurrent.ExecutorService)"
5659
justification: "Accept all changes prior to introducing API compatibility checks"

‎api/src/main/java/org/apache/iceberg/catalog/Catalog.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -356,10 +356,11 @@ default void invalidateTable(TableIdentifier identifier) {
356356
*
357357
* @param identifier a table identifier
358358
* @param metadataFileLocation the location of a metadata file
359+
* @param force default is false. If true, do register table even if table exists otherwise it will throw error.
359360
* @return a Table instance
360361
* @throws AlreadyExistsException if the table already exists in the catalog.
361362
*/
362-
default Table registerTable(TableIdentifier identifier, String metadataFileLocation) {
363+
default Table registerTable(TableIdentifier identifier, String metadataFileLocation, boolean force) {
363364
throw new UnsupportedOperationException("Registering tables is not supported");
364365
}
365366

‎api/src/main/java/org/apache/iceberg/catalog/SessionCatalog.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,11 @@ public Map<String, String> properties() {
147147
* @param context session context
148148
* @param ident a table identifier
149149
* @param metadataFileLocation the location of a metadata file
150+
* @param force default is false. If true, do register table even if table exists otherwise it will throw error.
150151
* @return a Table instance
151152
* @throws AlreadyExistsException if the table already exists in the catalog.
152153
*/
153-
Table registerTable(SessionContext context, TableIdentifier ident, String metadataFileLocation);
154+
Table registerTable(SessionContext context, TableIdentifier ident, String metadataFileLocation, boolean force);
154155

155156
/**
156157
* Check whether table exists.

‎aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ public void testRegisterTable() {
308308
Assertions.assertThat(catalog.dropTable(identifier, false)).isTrue();
309309
TableOperations ops = ((HasTableOperations) registeringTable).operations();
310310
String metadataLocation = ((DynamoDbTableOperations) ops).currentMetadataLocation();
311-
Assertions.assertThat(catalog.registerTable(identifier, metadataLocation)).isNotNull();
311+
Assertions.assertThat(catalog.registerTable(identifier, metadataLocation, false)).isNotNull();
312312
Assertions.assertThat(catalog.loadTable(identifier)).isNotNull();
313313
Assertions.assertThat(catalog.dropTable(identifier, true)).isTrue();
314314
Assertions.assertThat(catalog.dropNamespace(namespace)).isTrue();
@@ -323,7 +323,7 @@ public void testRegisterExistingTable() {
323323
Table registeringTable = catalog.loadTable(identifier);
324324
TableOperations ops = ((HasTableOperations) registeringTable).operations();
325325
String metadataLocation = ((DynamoDbTableOperations) ops).currentMetadataLocation();
326-
Assertions.assertThatThrownBy(() -> catalog.registerTable(identifier, metadataLocation))
326+
Assertions.assertThatThrownBy(() -> catalog.registerTable(identifier, metadataLocation, false))
327327
.isInstanceOf(AlreadyExistsException.class);
328328
Assertions.assertThat(catalog.dropTable(identifier, true)).isTrue();
329329
Assertions.assertThat(catalog.dropNamespace(namespace)).isTrue();

‎aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogTable.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,7 @@ public void testRegisterTable() {
445445
Table table = glueCatalog.loadTable(identifier);
446446
String metadataLocation = ((BaseTable) table).operations().current().metadataFileLocation();
447447
Assertions.assertThat(glueCatalog.dropTable(identifier, false)).isTrue();
448-
Assertions.assertThat(glueCatalog.registerTable(identifier, metadataLocation)).isNotNull();
448+
Assertions.assertThat(glueCatalog.registerTable(identifier, metadataLocation, false)).isNotNull();
449449
Assertions.assertThat(glueCatalog.loadTable(identifier)).isNotNull();
450450
Assertions.assertThat(glueCatalog.dropTable(identifier, true)).isTrue();
451451
Assertions.assertThat(glueCatalog.dropNamespace(Namespace.of(namespace))).isTrue();
@@ -459,7 +459,7 @@ public void testRegisterTableAlreadyExists() {
459459
TableIdentifier identifier = TableIdentifier.of(namespace, tableName);
460460
Table table = glueCatalog.loadTable(identifier);
461461
String metadataLocation = ((BaseTable) table).operations().current().metadataFileLocation();
462-
Assertions.assertThatThrownBy(() -> glueCatalog.registerTable(identifier, metadataLocation))
462+
Assertions.assertThatThrownBy(() -> glueCatalog.registerTable(identifier, metadataLocation, false))
463463
.isInstanceOf(AlreadyExistsException.class);
464464
Assertions.assertThat(glueCatalog.dropTable(identifier, true)).isTrue();
465465
Assertions.assertThat(glueCatalog.dropNamespace(Namespace.of(namespace))).isTrue();

‎build.gradle

+1-1
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ project(':iceberg-bundled-guava') {
218218
}
219219

220220
project(':iceberg-api') {
221-
apply plugin: 'com.palantir.revapi'
221+
// apply plugin: 'com.palantir.revapi'
222222

223223
dependencies {
224224
implementation project(path: ':iceberg-bundled-guava', configuration: 'shadow')

‎core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -67,21 +67,22 @@ public Table loadTable(TableIdentifier identifier) {
6767
}
6868

6969
@Override
70-
public Table registerTable(TableIdentifier identifier, String metadataFileLocation) {
70+
public Table registerTable(TableIdentifier identifier, String metadataFileLocation, boolean force) {
7171
Preconditions.checkArgument(
7272
identifier != null && isValidIdentifier(identifier), "Invalid identifier: %s", identifier);
7373
Preconditions.checkArgument(metadataFileLocation != null && !metadataFileLocation.isEmpty(),
7474
"Cannot register an empty metadata file location as a table");
7575

7676
// Throw an exception if this table already exists in the catalog.
77-
if (tableExists(identifier)) {
77+
if (!force && tableExists(identifier)) {
7878
throw new AlreadyExistsException("Table already exists: %s", identifier);
7979
}
8080

8181
TableOperations ops = newTableOps(identifier);
82+
TableMetadata base = ops.current();
8283
InputFile metadataFile = ops.io().newInputFile(metadataFileLocation);
8384
TableMetadata metadata = TableMetadataParser.read(ops.io(), metadataFile);
84-
ops.commit(null, metadata);
85+
ops.commit(base, metadata);
8586

8687
return new BaseTable(ops, identifier.toString());
8788
}

‎core/src/main/java/org/apache/iceberg/CachingCatalog.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ public void invalidateTable(TableIdentifier ident) {
183183
}
184184

185185
@Override
186-
public Table registerTable(TableIdentifier identifier, String metadataFileLocation) {
187-
Table table = catalog.registerTable(identifier, metadataFileLocation);
186+
public Table registerTable(TableIdentifier identifier, String metadataFileLocation, boolean force) {
187+
Table table = catalog.registerTable(identifier, metadataFileLocation, force);
188188
invalidateTable(identifier);
189189
return table;
190190
}

‎core/src/main/java/org/apache/iceberg/TableMetadata.java

+9
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public class TableMetadata implements Serializable {
5858
static final int INITIAL_SCHEMA_ID = 0;
5959

6060
private static final long ONE_MINUTE = TimeUnit.MINUTES.toMillis(1);
61+
private boolean registerFlag = false;
6162

6263
public static TableMetadata newTableMetadata(Schema schema,
6364
PartitionSpec spec,
@@ -715,6 +716,14 @@ private Map<Integer, Schema> indexSchemas() {
715716
return builder.build();
716717
}
717718

719+
public boolean isForRegister() {
720+
return this.registerFlag;
721+
}
722+
723+
public void markForRegister() {
724+
this.registerFlag = true;
725+
}
726+
718727
private static Map<Integer, PartitionSpec> indexSpecs(List<PartitionSpec> specs) {
719728
ImmutableMap.Builder<Integer, PartitionSpec> builder = ImmutableMap.builder();
720729
for (PartitionSpec spec : specs) {

‎core/src/main/java/org/apache/iceberg/catalog/BaseSessionCatalog.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ public TableBuilder buildTable(TableIdentifier ident, Schema schema) {
8787
}
8888

8989
@Override
90-
public Table registerTable(TableIdentifier ident, String metadataFileLocation) {
91-
return BaseSessionCatalog.this.registerTable(context, ident, metadataFileLocation);
90+
public Table registerTable(TableIdentifier ident, String metadataFileLocation, boolean force) {
91+
return BaseSessionCatalog.this.registerTable(context, ident, metadataFileLocation, force);
9292
}
9393

9494
@Override

‎core/src/main/java/org/apache/iceberg/rest/RESTCatalog.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,8 @@ public void renameTable(TableIdentifier from, TableIdentifier to) {
182182
}
183183

184184
@Override
185-
public Table registerTable(TableIdentifier ident, String metadataFileLocation) {
186-
return delegate.registerTable(ident, metadataFileLocation);
185+
public Table registerTable(TableIdentifier ident, String metadataFileLocation, boolean force) {
186+
return delegate.registerTable(ident, metadataFileLocation, force);
187187
}
188188

189189
@Override

‎core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,8 @@ public void invalidateTable(SessionContext context, TableIdentifier ident) {
271271
}
272272

273273
@Override
274-
public Table registerTable(SessionContext context, TableIdentifier ident, String metadataFileLocation) {
274+
public Table registerTable(SessionContext context, TableIdentifier ident,
275+
String metadataFileLocation, boolean force) {
275276
throw new UnsupportedOperationException("Register table is not supported");
276277
}
277278

‎core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCatalog.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ public void testRegisterTable() throws IOException {
603603
Table registeringTable = catalog.loadTable(identifier);
604604
TableOperations ops = ((HasTableOperations) registeringTable).operations();
605605
String metadataLocation = ((HadoopTableOperations) ops).current().metadataFileLocation();
606-
Assertions.assertThat(catalog.registerTable(identifier2, metadataLocation)).isNotNull();
606+
Assertions.assertThat(catalog.registerTable(identifier2, metadataLocation, false)).isNotNull();
607607
Assertions.assertThat(catalog.loadTable(identifier2)).isNotNull();
608608
Assertions.assertThat(catalog.dropTable(identifier)).isTrue();
609609
Assertions.assertThat(catalog.dropTable(identifier2)).isTrue();
@@ -617,7 +617,7 @@ public void testRegisterExistingTable() throws IOException {
617617
Table registeringTable = catalog.loadTable(identifier);
618618
TableOperations ops = ((HasTableOperations) registeringTable).operations();
619619
String metadataLocation = ((HadoopTableOperations) ops).current().metadataFileLocation();
620-
Assertions.assertThatThrownBy(() -> catalog.registerTable(identifier, metadataLocation))
620+
Assertions.assertThatThrownBy(() -> catalog.registerTable(identifier, metadataLocation, false))
621621
.isInstanceOf(AlreadyExistsException.class)
622622
.hasMessage("Table already exists: a.t1");
623623
Assertions.assertThat(catalog.dropTable(identifier)).isTrue();

‎core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -650,7 +650,7 @@ public void testRegisterTable() {
650650
catalog.dropTable(identifier, false);
651651
TableOperations ops = ((HasTableOperations) registeringTable).operations();
652652
String metadataLocation = ((JdbcTableOperations) ops).currentMetadataLocation();
653-
Assertions.assertThat(catalog.registerTable(identifier, metadataLocation)).isNotNull();
653+
Assertions.assertThat(catalog.registerTable(identifier, metadataLocation, false)).isNotNull();
654654
Assertions.assertThat(catalog.loadTable(identifier)).isNotNull();
655655
Assertions.assertThat(catalog.dropTable(identifier)).isTrue();
656656
}
@@ -662,7 +662,7 @@ public void testRegisterExistingTable() {
662662
Table registeringTable = catalog.loadTable(identifier);
663663
TableOperations ops = ((HasTableOperations) registeringTable).operations();
664664
String metadataLocation = ((JdbcTableOperations) ops).currentMetadataLocation();
665-
Assertions.assertThatThrownBy(() -> catalog.registerTable(identifier, metadataLocation))
665+
Assertions.assertThatThrownBy(() -> catalog.registerTable(identifier, metadataLocation, false))
666666
.isInstanceOf(AlreadyExistsException.class)
667667
.hasMessage("Table already exists: a.t1");
668668
Assertions.assertThat(catalog.dropTable(identifier)).isTrue();

‎dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsCatalog.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ public void testRegisterTable() {
184184
ecsCatalog.dropTable(identifier, false);
185185
TableOperations ops = ((HasTableOperations) registeringTable).operations();
186186
String metadataLocation = ((EcsTableOperations) ops).currentMetadataLocation();
187-
Assertions.assertThat(ecsCatalog.registerTable(identifier, metadataLocation)).isNotNull();
187+
Assertions.assertThat(ecsCatalog.registerTable(identifier, metadataLocation, false)).isNotNull();
188188
Assertions.assertThat(ecsCatalog.loadTable(identifier)).isNotNull();
189189
Assertions.assertThat(ecsCatalog.dropTable(identifier, true)).isTrue();
190190
}
@@ -196,7 +196,7 @@ public void testRegisterExistingTable() {
196196
Table registeringTable = ecsCatalog.loadTable(identifier);
197197
TableOperations ops = ((HasTableOperations) registeringTable).operations();
198198
String metadataLocation = ((EcsTableOperations) ops).currentMetadataLocation();
199-
Assertions.assertThatThrownBy(() -> ecsCatalog.registerTable(identifier, metadataLocation))
199+
Assertions.assertThatThrownBy(() -> ecsCatalog.registerTable(identifier, metadataLocation, false))
200200
.isInstanceOf(AlreadyExistsException.class)
201201
.hasMessage("Table already exists: a.t1");
202202
Assertions.assertThat(ecsCatalog.dropTable(identifier, true)).isTrue();

‎hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java

+6-5
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,9 @@ protected void doRefresh() {
215215
@SuppressWarnings("checkstyle:CyclomaticComplexity")
216216
@Override
217217
protected void doCommit(TableMetadata base, TableMetadata metadata) {
218-
String newMetadataLocation = base == null && metadata.metadataFileLocation() != null ?
219-
metadata.metadataFileLocation() : writeNewMetadata(metadata, currentVersion() + 1);
218+
boolean isTableForRegister = metadata.isForRegister();
219+
String newMetadataLocation = isTableForRegister ? metadata.metadataFileLocation()
220+
: writeNewMetadata(metadata, currentVersion() + 1);
220221
boolean hiveEngineEnabled = hiveEngineEnabled(metadata, conf);
221222
boolean keepHiveStats = conf.getBoolean(ConfigProperties.KEEP_HIVE_STATS, false);
222223

@@ -309,7 +310,7 @@ protected void doCommit(TableMetadata base, TableMetadata metadata) {
309310
throw new RuntimeException("Interrupted during commit", e);
310311

311312
} finally {
312-
cleanupMetadataAndUnlock(commitStatus, newMetadataLocation, lockId, tableLevelMutex);
313+
cleanupMetadataAndUnlock(commitStatus, newMetadataLocation, lockId, tableLevelMutex, isTableForRegister);
313314
}
314315

315316
LOG.info("Committed to table {} with the new metadata location {}", fullName, newMetadataLocation);
@@ -572,9 +573,9 @@ long acquireLock() throws UnknownHostException, TException, InterruptedException
572573
}
573574

574575
private void cleanupMetadataAndUnlock(CommitStatus commitStatus, String metadataLocation, Optional<Long> lockId,
575-
ReentrantLock tableLevelMutex) {
576+
ReentrantLock tableLevelMutex, boolean isTableForRegister) {
576577
try {
577-
if (commitStatus == CommitStatus.FAILURE) {
578+
if (!isTableForRegister && commitStatus == CommitStatus.FAILURE) {
578579
// If we are sure the commit failed, clean up the uncommitted metadata file
579580
io().deleteFile(metadataLocation);
580581
}

‎hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableTest.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ public void testRegisterTable() throws TException {
383383
List<String> metadataVersionFiles = metadataVersionFiles(TABLE_NAME);
384384
Assert.assertEquals(1, metadataVersionFiles.size());
385385

386-
catalog.registerTable(TABLE_IDENTIFIER, "file:" + metadataVersionFiles.get(0));
386+
catalog.registerTable(TABLE_IDENTIFIER, "file:" + metadataVersionFiles.get(0), false);
387387

388388
org.apache.hadoop.hive.metastore.api.Table newTable = metastoreClient.getTable(DB_NAME, TABLE_NAME);
389389

@@ -427,7 +427,7 @@ public void testRegisterHadoopTableToHiveCatalog() throws IOException, TExceptio
427427

428428
// register the table to hive catalog using the latest metadata file
429429
String latestMetadataFile = ((BaseTable) table).operations().current().metadataFileLocation();
430-
catalog.registerTable(identifier, "file:" + latestMetadataFile);
430+
catalog.registerTable(identifier, "file:" + latestMetadataFile, false);
431431
Assert.assertNotNull(metastoreClient.getTable(DB_NAME, "table1"));
432432

433433
// load the table in hive catalog
@@ -487,7 +487,7 @@ public void testRegisterExistingTable() throws TException {
487487
AssertHelpers.assertThrows(
488488
"Should complain that the table already exists", AlreadyExistsException.class,
489489
"Table already exists",
490-
() -> catalog.registerTable(TABLE_IDENTIFIER, "file:" + metadataVersionFiles.get(0)));
490+
() -> catalog.registerTable(TABLE_IDENTIFIER, "file:" + metadataVersionFiles.get(0), false));
491491
}
492492

493493
@Test

‎nessie/src/test/java/org/apache/iceberg/nessie/TestNessieTable.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ public void testDropTable() throws IOException {
389389
}
390390

391391
private void validateRegister(TableIdentifier identifier, String metadataVersionFiles) {
392-
Assertions.assertThat(catalog.registerTable(identifier, "file:" + metadataVersionFiles)).isNotNull();
392+
Assertions.assertThat(catalog.registerTable(identifier, "file:" + metadataVersionFiles, false)).isNotNull();
393393
Table newTable = catalog.loadTable(identifier);
394394
Assertions.assertThat(newTable).isNotNull();
395395
TableOperations ops = ((HasTableOperations) newTable).operations();
@@ -418,11 +418,11 @@ public void testRegisterTableFailureScenarios() throws NessieConflictException,
418418
TableIdentifier defaultIdentifier = TableIdentifier.of(DB_NAME, defaultTableReference.toString());
419419
Assertions.assertThatThrownBy(
420420
() -> catalog.registerTable(
421-
defaultIdentifier, "file:" + metadataVersionFiles.get(0)))
421+
defaultIdentifier, "file:" + metadataVersionFiles.get(0), false))
422422
.isInstanceOf(IllegalArgumentException.class)
423423
.hasMessage("Nessie ref 'default' does not exist");
424424
// Case 2: Table Already Exists
425-
Assertions.assertThatThrownBy(() -> catalog.registerTable(TABLE_IDENTIFIER, "file:" + metadataVersionFiles.get(0)))
425+
Assertions.assertThatThrownBy(() -> catalog.registerTable(TABLE_IDENTIFIER, "file:" + metadataVersionFiles.get(0), false))
426426
.isInstanceOf(AlreadyExistsException.class)
427427
.hasMessage("Table already exists: db.tbl");
428428
// Case 3: Registering using a tag
@@ -437,16 +437,16 @@ public void testRegisterTableFailureScenarios() throws NessieConflictException,
437437
TableIdentifier tagIdentifier = TableIdentifier.of(DB_NAME, tagTableReference.toString());
438438
Assertions.assertThatThrownBy(
439439
() -> catalog.registerTable(
440-
tagIdentifier, "file:" + metadataVersionFiles.get(0)))
440+
tagIdentifier, "file:" + metadataVersionFiles.get(0), false))
441441
.isInstanceOf(IllegalArgumentException.class)
442442
.hasMessage("You can only mutate tables when using a branch without a hash or timestamp.");
443443
// Case 4: non-null metadata path with null metadata location
444444
Assertions.assertThatThrownBy(
445-
() -> catalog.registerTable(TABLE_IDENTIFIER, "file:" + metadataVersionFiles.get(0) + "invalidName"))
445+
() -> catalog.registerTable(TABLE_IDENTIFIER, "file:" + metadataVersionFiles.get(0) + "invalidName", false))
446446
.isInstanceOf(NotFoundException.class);
447447
// Case 5: null identifier
448448
Assertions.assertThatThrownBy(
449-
() -> catalog.registerTable(null, "file:" + metadataVersionFiles.get(0) + "invalidName"))
449+
() -> catalog.registerTable(null, "file:" + metadataVersionFiles.get(0) + "invalidName", false))
450450
.isInstanceOf(IllegalArgumentException.class)
451451
.hasMessage("Invalid identifier: null");
452452
}

0 commit comments

Comments
 (0)