Skip to content

Commit 448b3d4

Browse files
KodaiDKodaiD
committed
Fix to handle rename in metadata correctly
1 parent 7a396bb commit 448b3d4

File tree

5 files changed

+201
-23
lines changed

5 files changed

+201
-23
lines changed

‎core/src/main/java/com/scalar/db/api/TableMetadata.java‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,33 @@ public Builder removeColumn(String name) {
251251
return this;
252252
}
253253

254+
/**
255+
* Renames a column from {@code oldName} to {@code newName}.
256+
*
257+
* @param oldName a column name to be renamed
258+
* @param newName a new column name
259+
* @return a builder instance
260+
*/
261+
public Builder renameColumn(String oldName, String newName) {
262+
if (columns.containsKey(oldName)) {
263+
LinkedHashMap<String, DataType> newColumns = new LinkedHashMap<>();
264+
for (Map.Entry<String, DataType> entry : columns.entrySet()) {
265+
if (entry.getKey().equals(oldName)) {
266+
newColumns.put(newName, entry.getValue());
267+
} else {
268+
newColumns.put(entry.getKey(), entry.getValue());
269+
}
270+
}
271+
columns.clear();
272+
columns.putAll(newColumns);
273+
}
274+
if (encryptedColumnNames.contains(oldName)) {
275+
encryptedColumnNames.remove(oldName);
276+
encryptedColumnNames.add(newName);
277+
}
278+
return this;
279+
}
280+
254281
/**
255282
* Adds a partition-key column with the specified name.
256283
*
@@ -273,6 +300,29 @@ public Builder removePartitionKey(String name) {
273300
return this;
274301
}
275302

303+
/**
304+
* Renames a partition-key column from {@code oldName} to {@code newName}.
305+
*
306+
* @param oldName a column name to be renamed
307+
* @param newName a new column name
308+
* @return a builder instance
309+
*/
310+
public Builder renamePartitionKey(String oldName, String newName) {
311+
if (partitionKeyNames.contains(oldName)) {
312+
LinkedHashSet<String> newPartitionKeyNames = new LinkedHashSet<>();
313+
for (String name : partitionKeyNames) {
314+
if (name.equals(oldName)) {
315+
newPartitionKeyNames.add(newName);
316+
} else {
317+
newPartitionKeyNames.add(name);
318+
}
319+
}
320+
partitionKeyNames.clear();
321+
partitionKeyNames.addAll(newPartitionKeyNames);
322+
}
323+
return this;
324+
}
325+
276326
/**
277327
* Adds a clustering-key column with the specified name and the default order (ASC).
278328
*
@@ -309,6 +359,34 @@ public Builder removeClusteringKey(String name) {
309359
return this;
310360
}
311361

362+
/**
363+
* Renames a clustering-key column from {@code oldName} to {@code newName}.
364+
*
365+
* @param oldName a column name to be renamed
366+
* @param newName a new column name
367+
* @return a builder instance
368+
*/
369+
public Builder renameClusteringKey(String oldName, String newName) {
370+
if (clusteringKeyNames.contains(oldName)) {
371+
LinkedHashSet<String> newClusteringKeyNames = new LinkedHashSet<>();
372+
LinkedHashMap<String, Order> newClusteringOrders = new LinkedHashMap<>();
373+
for (String name : clusteringKeyNames) {
374+
if (name.equals(oldName)) {
375+
newClusteringKeyNames.add(newName);
376+
newClusteringOrders.put(newName, clusteringOrders.get(oldName));
377+
} else {
378+
newClusteringKeyNames.add(name);
379+
newClusteringOrders.put(name, clusteringOrders.get(name));
380+
}
381+
}
382+
clusteringKeyNames.clear();
383+
clusteringOrders.clear();
384+
clusteringKeyNames.addAll(newClusteringKeyNames);
385+
clusteringOrders.putAll(newClusteringOrders);
386+
}
387+
return this;
388+
}
389+
312390
/**
313391
* Adds a secondary-index column with the specified name.
314392
*
@@ -331,6 +409,21 @@ public Builder removeSecondaryIndex(String name) {
331409
return this;
332410
}
333411

412+
/**
413+
* Renames a secondary-index column from {@code oldName} to {@code newName}.
414+
*
415+
* @param oldName a column name to be renamed
416+
* @param newName a new column name
417+
* @return a builder instance
418+
*/
419+
public Builder renameSecondaryIndex(String oldName, String newName) {
420+
if (secondaryIndexNames.contains(oldName)) {
421+
secondaryIndexNames.remove(oldName);
422+
secondaryIndexNames.add(newName);
423+
}
424+
return this;
425+
}
426+
334427
/**
335428
* Builds a TableMetadata instance.
336429
*

‎core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -891,21 +891,14 @@ public void renameColumn(
891891
try {
892892
TableMetadata currentTableMetadata = getTableMetadata(namespace, table);
893893
assert currentTableMetadata != null;
894-
DataType columnType = currentTableMetadata.getColumnDataType(oldColumnName);
895894
TableMetadata.Builder tableMetadataBuilder =
896-
TableMetadata.newBuilder(currentTableMetadata)
897-
.removeColumn(oldColumnName)
898-
.addColumn(newColumnName, columnType);
895+
TableMetadata.newBuilder(currentTableMetadata).renameColumn(oldColumnName, newColumnName);
899896
if (currentTableMetadata.getPartitionKeyNames().contains(oldColumnName)) {
900-
tableMetadataBuilder.removePartitionKey(oldColumnName);
901-
tableMetadataBuilder.addPartitionKey(newColumnName);
897+
tableMetadataBuilder.renamePartitionKey(oldColumnName, newColumnName);
902898
} else if (currentTableMetadata.getClusteringKeyNames().contains(oldColumnName)) {
903-
Ordering.Order order = currentTableMetadata.getClusteringOrder(oldColumnName);
904-
tableMetadataBuilder.removeClusteringKey(oldColumnName);
905-
tableMetadataBuilder.addClusteringKey(newColumnName, order);
899+
tableMetadataBuilder.renameClusteringKey(oldColumnName, newColumnName);
906900
} else if (currentTableMetadata.getSecondaryIndexNames().contains(oldColumnName)) {
907-
tableMetadataBuilder.removeSecondaryIndex(oldColumnName);
908-
tableMetadataBuilder.addSecondaryIndex(newColumnName);
901+
tableMetadataBuilder.renameSecondaryIndex(oldColumnName, newColumnName);
909902
}
910903
TableMetadata updatedTableMetadata = tableMetadataBuilder.build();
911904
String renameColumnStatement =

‎core/src/test/java/com/scalar/db/api/TableMetadataTest.java‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,4 +263,80 @@ public void builder_basedOnAnotherTableMetadata_ShouldReturnProperTableMetadata(
263263

264264
assertThat(tableMetadata.getEncryptedColumnNames()).containsOnly(COL_NAME10);
265265
}
266+
267+
@Test
268+
public void builder_RenameColumns_ShouldRenameColumnsInTableMetadataCorrectly() {
269+
// Arrange
270+
TableMetadata.Builder builder =
271+
TableMetadata.newBuilder()
272+
.addColumn(COL_NAME1, DataType.INT)
273+
.addColumn(COL_NAME2, DataType.TEXT)
274+
.addColumn(COL_NAME3, DataType.TEXT)
275+
.addColumn(COL_NAME4, DataType.INT)
276+
.addColumn(COL_NAME5, DataType.INT)
277+
.addColumn(COL_NAME6, DataType.TEXT)
278+
.addColumn(COL_NAME7, DataType.BIGINT)
279+
.addColumn(COL_NAME8, DataType.FLOAT)
280+
.addColumn(COL_NAME9, DataType.DOUBLE)
281+
.addColumn(COL_NAME10, DataType.BOOLEAN, true)
282+
.addColumn(COL_NAME11, DataType.BLOB, true)
283+
.addPartitionKey(COL_NAME2)
284+
.addPartitionKey(COL_NAME1)
285+
.addClusteringKey(COL_NAME4, Order.ASC)
286+
.addClusteringKey(COL_NAME3, Order.DESC);
287+
288+
// Act
289+
TableMetadata tableMetadata =
290+
builder
291+
.renameColumn(COL_NAME2, "new_" + COL_NAME2)
292+
.renameColumn(COL_NAME4, "new_" + COL_NAME4)
293+
.renameColumn(COL_NAME10, "new_" + COL_NAME10)
294+
.renamePartitionKey(COL_NAME2, "new_" + COL_NAME2)
295+
.renameClusteringKey(COL_NAME4, "new_" + COL_NAME4)
296+
.build();
297+
298+
// Assert
299+
assertThat(tableMetadata.getColumnNames().size()).isEqualTo(11);
300+
assertThat(tableMetadata.getColumnNames().contains(COL_NAME2)).isFalse();
301+
assertThat(tableMetadata.getColumnNames().contains("new_" + COL_NAME2)).isTrue();
302+
assertThat(tableMetadata.getColumnNames().contains(COL_NAME4)).isFalse();
303+
assertThat(tableMetadata.getColumnNames().contains("new_" + COL_NAME4)).isTrue();
304+
assertThat(tableMetadata.getColumnNames().contains(COL_NAME10)).isFalse();
305+
assertThat(tableMetadata.getColumnNames().contains("new_" + COL_NAME10)).isTrue();
306+
assertThat(tableMetadata.getPartitionKeyNames().contains(COL_NAME2)).isFalse();
307+
assertThat(tableMetadata.getPartitionKeyNames().contains("new_" + COL_NAME2)).isTrue();
308+
assertThat(tableMetadata.getClusteringKeyNames().contains(COL_NAME4)).isFalse();
309+
assertThat(tableMetadata.getClusteringKeyNames().contains("new_" + COL_NAME4)).isTrue();
310+
assertThat(tableMetadata.getEncryptedColumnNames().contains(COL_NAME10)).isFalse();
311+
assertThat(tableMetadata.getEncryptedColumnNames().contains("new_" + COL_NAME10)).isTrue();
312+
}
313+
314+
@Test
315+
public void builder_RenameSecondaryIndex_ShouldRenameSecondaryIndexInTableMetadataCorrectly() {
316+
// Arrange
317+
TableMetadata.Builder builder =
318+
TableMetadata.newBuilder()
319+
.addColumn(COL_NAME1, DataType.INT)
320+
.addColumn(COL_NAME2, DataType.TEXT)
321+
.addColumn(COL_NAME3, DataType.TEXT)
322+
.addColumn(COL_NAME4, DataType.INT)
323+
.addColumn(COL_NAME5, DataType.INT)
324+
.addColumn(COL_NAME6, DataType.TEXT)
325+
.addPartitionKey(COL_NAME2)
326+
.addPartitionKey(COL_NAME1)
327+
.addClusteringKey(COL_NAME4, Order.ASC)
328+
.addClusteringKey(COL_NAME3, Order.DESC)
329+
.addSecondaryIndex(COL_NAME5)
330+
.addSecondaryIndex(COL_NAME6);
331+
332+
// Act
333+
TableMetadata tableMetadata =
334+
builder.renameSecondaryIndex(COL_NAME5, "new_" + COL_NAME5).build();
335+
336+
// Assert
337+
assertThat(tableMetadata.getSecondaryIndexNames().size()).isEqualTo(2);
338+
assertThat(tableMetadata.getSecondaryIndexNames().contains(COL_NAME5)).isFalse();
339+
assertThat(tableMetadata.getSecondaryIndexNames().contains("new_" + COL_NAME5)).isTrue();
340+
assertThat(tableMetadata.getSecondaryIndexNames().contains(COL_NAME6)).isTrue();
341+
}
266342
}

‎integration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,23 +1222,31 @@ public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly()
12221222
.addColumn(getColumnName1(), DataType.INT)
12231223
.addColumn(getColumnName2(), DataType.INT)
12241224
.addColumn(getColumnName3(), DataType.TEXT)
1225+
.addColumn(getColumnName4(), DataType.INT)
1226+
.addColumn(getColumnName5(), DataType.INT)
12251227
.addPartitionKey(getColumnName1())
1226-
.addClusteringKey(getColumnName2())
1228+
.addPartitionKey(getColumnName2())
1229+
.addClusteringKey(getColumnName3())
1230+
.addClusteringKey(getColumnName4())
12271231
.build();
12281232
admin.createTable(namespace1, getTable4(), currentTableMetadata, options);
12291233

12301234
// Act
1231-
admin.renameColumn(namespace1, getTable4(), getColumnName1(), getColumnName4());
1232-
admin.renameColumn(namespace1, getTable4(), getColumnName2(), getColumnName5());
1235+
admin.renameColumn(namespace1, getTable4(), getColumnName1(), getColumnName6());
1236+
admin.renameColumn(namespace1, getTable4(), getColumnName3(), getColumnName7());
12331237

12341238
// Assert
12351239
TableMetadata expectedTableMetadata =
12361240
TableMetadata.newBuilder()
1241+
.addColumn(getColumnName6(), DataType.INT)
1242+
.addColumn(getColumnName2(), DataType.INT)
1243+
.addColumn(getColumnName7(), DataType.TEXT)
12371244
.addColumn(getColumnName4(), DataType.INT)
12381245
.addColumn(getColumnName5(), DataType.INT)
1239-
.addColumn(getColumnName3(), DataType.TEXT)
1240-
.addPartitionKey(getColumnName4())
1241-
.addClusteringKey(getColumnName5())
1246+
.addPartitionKey(getColumnName6())
1247+
.addPartitionKey(getColumnName2())
1248+
.addClusteringKey(getColumnName7())
1249+
.addClusteringKey(getColumnName4())
12421250
.build();
12431251
assertThat(admin.getTableMetadata(namespace1, getTable4())).isEqualTo(expectedTableMetadata);
12441252
} finally {

‎integration-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,23 +1090,31 @@ public void renameColumn_ForPrimaryKeyColumn_ShouldRenameColumnCorrectly()
10901090
.addColumn("c1", DataType.INT)
10911091
.addColumn("c2", DataType.INT)
10921092
.addColumn("c3", DataType.TEXT)
1093+
.addColumn("c4", DataType.INT)
1094+
.addColumn("c5", DataType.INT)
10931095
.addPartitionKey("c1")
1094-
.addClusteringKey("c2")
1096+
.addPartitionKey("c2")
1097+
.addClusteringKey("c3")
1098+
.addClusteringKey("c4")
10951099
.build();
10961100
admin.createTable(namespace1, TABLE4, currentTableMetadata, options);
10971101

10981102
// Act
1099-
admin.renameColumn(namespace1, TABLE4, "c1", "c4");
1100-
admin.renameColumn(namespace1, TABLE4, "c2", "c5");
1103+
admin.renameColumn(namespace1, TABLE4, "c1", "c6");
1104+
admin.renameColumn(namespace1, TABLE4, "c3", "c7");
11011105

11021106
// Assert
11031107
TableMetadata expectedTableMetadata =
11041108
TableMetadata.newBuilder()
1109+
.addColumn("c6", DataType.INT)
1110+
.addColumn("c2", DataType.INT)
1111+
.addColumn("c7", DataType.TEXT)
11051112
.addColumn("c4", DataType.INT)
11061113
.addColumn("c5", DataType.INT)
1107-
.addColumn("c3", DataType.TEXT)
1108-
.addPartitionKey("c4")
1109-
.addClusteringKey("c5")
1114+
.addPartitionKey("c6")
1115+
.addPartitionKey("c2")
1116+
.addClusteringKey("c7")
1117+
.addClusteringKey("c4")
11101118
.build();
11111119
assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata);
11121120
} finally {

0 commit comments

Comments
 (0)