kfaraz commented on code in PR #18515:
URL: https://github.com/apache/druid/pull/18515#discussion_r2377842026
##########
server/src/test/java/org/apache/druid/metadata/SQLMetadataConnectorTest.java:
##########
@@ -305,6 +307,170 @@ public void testIsTransientException()
);
}
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void test_tableIndices_areNotReplaced_ifExist(boolean
useUniqueIndexName)
+ {
+ tablesConfig = new MetadataStorageTablesConfig(
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ useUniqueIndexName
+ );
+ connector = new TestDerbyConnector(new MetadataStorageConnectorConfig(),
tablesConfig);
+
+ final String segmentsTable = tablesConfig.getSegmentsTable();
+ final List<String> legacyIndexTemplates = Arrays.asList(
+ "idx_%1$s_used",
+ "idx_%1$s_datasource_used_end_start",
+ "idx_%1$s_datasource_upgraded_from_segment_id"
+ );
+ final List<List<String>> indexColumns = List.of(
+ List.of("used"),
+ List.of(
+ "dataSource",
+ "used",
+ StringUtils.format("%send%S", connector.getQuoteString(),
connector.getQuoteString()),
+ "start"
+ ),
+ List.of("dataSource", "upgraded_from_segment_id")
+ );
+ final List<String> legacyIndexNames = legacyIndexTemplates.stream()
+ .map(template ->
StringUtils.toUpperCase(StringUtils.format(
+ template,
+ segmentsTable
+ )))
+
.collect(Collectors.toList());
+ final List<String> uniqueIndexNames = indexColumns.stream()
+ .map(cols ->
connector.generateUniqueIndexName(
+ segmentsTable,
+ cols
+ ))
+
.collect(Collectors.toList());
+
+ connector.createSegmentTable(segmentsTable);
+ connector.alterSegmentTable();
+
+ // drop legacy/unique indices and create the other type
+ connector.getDBI().withHandle(handle -> {
+ for (int i = 0; i < legacyIndexNames.size(); i++) {
+ handle.execute(StringUtils.toUpperCase(StringUtils.format(
+ "DROP INDEX %s",
+ useUniqueIndexName ? uniqueIndexNames.get(i) :
legacyIndexNames.get(i)
+ )));
+ handle.execute(StringUtils.format(
+ "CREATE INDEX %s ON %s(%s)",
+ useUniqueIndexName ? legacyIndexNames.get(i) :
uniqueIndexNames.get(i),
+ segmentsTable,
+ String.join(",", indexColumns.get(i))
+ ));
+ }
+ return null;
+ });
+
+ // run again to index name checks are run and duplicate indices are not
created
+ connector.createSegmentTable(segmentsTable);
+ connector.alterSegmentTable();
+ assertIndicesPresentOnTable(segmentsTable, useUniqueIndexName ?
legacyIndexNames : uniqueIndexNames);
+
+ dropTable(segmentsTable);
+ connector.tearDown();
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void test_tableIndices_areReplaced_IfNotExist(boolean
useUniqueIndexName)
+ {
+ tablesConfig = new MetadataStorageTablesConfig(
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ null,
+ useUniqueIndexName
+ );
+ connector = new TestDerbyConnector(new MetadataStorageConnectorConfig(),
tablesConfig);
+
+ final String segmentsTable = tablesConfig.getSegmentsTable();
+ final List<String> legacyIndexTemplates = Arrays.asList(
+ "idx_%1$s_used",
+ "idx_%1$s_datasource_used_end_start",
+ "idx_%1$s_datasource_upgraded_from_segment_id"
+ );
+ final List<List<String>> indexColumns = List.of(
+ List.of("used"),
+ List.of(
+ "dataSource",
+ "used",
+ StringUtils.format("%send%S", connector.getQuoteString(),
connector.getQuoteString()),
+ "start"
+ ),
+ List.of("dataSource", "upgraded_from_segment_id")
+ );
Review Comment:
Since this is a test setup, we shouldn't have to go through the whole flow
of generating index names using formatters etc.
Instead, we should verify the actual index names. Something like:
```java
connector.createSegmentsTable(...);
verifyIndicesPresentOnTable(
"IDX_ABC",
"IDX_DEF",
"IDX_XYZ
);
```
This would make the tests much more readable and exposes the end result
while hiding the impl details.
It is okay even if we have to change the test when the logic in the
`SQLMetadataConnector.generateIndexName()` method changes
(in fact, it is desirable to have to update the test in such cases).
--
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]
