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.\
   
   Instead, we should just do something like:
   
   ```java
   connector.createSegmentsTable(...);
   
   verifyIndicesPresentOnTable(
      "IDX_ABC",
      "IDX_DEF",
      "IDX_XYZ
   );
   
   ```
   
   This would make the tests much more readable.
   It is okay even if we have to change the test when the logic in the 
`SQLMetadataConnector.generateIndexName()` (in fact, it is desirable).



-- 
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]

Reply via email to