jtuglu1 commented on code in PR #18515:
URL: https://github.com/apache/druid/pull/18515#discussion_r2369219765
##########
server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java:
##########
@@ -1106,41 +1168,48 @@ public ResultSet getIndexInfo(DatabaseMetaData
databaseMetaData, String tableNam
}
/**
- * create index on the table with retry if not already exist, to be called
after createTable
+ * Create index on the table with retry if not already exist, to be called
after createTable
+ * Format of index name is either specified via legacy {@param
legacyIndexNameFormat} or {@code generateSHABasedIndexIdentifier}.
*
* @param tableName Name of the table to create index on
- * @param indexName case-insensitive string index name, it helps to
check the existing index on table
- * @param indexCols List of columns to be indexed on
+ * @param legacyIndexNameFormat Template to create index ID (nullable for
forwards compatibility with SHA-based indices)
+ * @param indexCols List of un-escaped columns to be indexed on
(case-sensitive).
* @param createdIndexSet
*/
public void createIndex(
final String tableName,
- final String indexName,
+ final String legacyIndexNameFormat,
final List<String> indexCols,
final Set<String> createdIndexSet
)
{
+ final String shaIndexName =
StringUtils.toUpperCase(generateUniqueIndexName(tableName, indexCols));
+ final String legacyIndexName = legacyIndexNameFormat != null
+ ?
StringUtils.toUpperCase(StringUtils.format(legacyIndexNameFormat, tableName))
+ : null;
+
+ // Avoid creating duplicate indices if an index with either naming
convention already exists
+ if (legacyIndexName != null && createdIndexSet.contains(legacyIndexName)) {
+ log.info("Legacy index[%s] on Table[%s] already exists",
legacyIndexName, tableName);
+ return;
+ } else if (createdIndexSet.contains(shaIndexName)) {
+ log.info("Index[%s] on Table[%s] already exists", shaIndexName,
tableName);
+ return;
+ }
+
+ final String indexName =
tablesConfigSupplier.get().getUseUniqueIndexNames() ? shaIndexName :
legacyIndexName;
try {
retryWithHandle(
- new HandleCallback<Void>()
- {
- @Override
- public Void withHandle(Handle handle)
- {
- if
(!createdIndexSet.contains(StringUtils.toUpperCase(indexName))) {
- String indexSQL = StringUtils.format(
- "CREATE INDEX %1$s ON %2$s(%3$s)",
- indexName,
- tableName,
- Joiner.on(",").join(indexCols)
- );
- log.info("Creating Index on Table [%s], sql: [%s] ",
tableName, indexSQL);
- handle.execute(indexSQL);
- } else {
- log.info("Index [%s] on Table [%s] already exists", indexName,
tableName);
- }
- return null;
- }
+ (HandleCallback<Void>) handle -> {
+ String indexSQL = StringUtils.format(
+ "CREATE INDEX %1$s ON %2$s(%3$s)",
+ indexName,
+ tableName,
+ Joiner.on(",").join(indexCols)
+ );
+ log.info("Creating index[%s] on table[%s] using SQL[%s].",
indexName, tableName, indexSQL);
+ handle.execute(indexSQL);
+ return null;
}
);
Review Comment:
For completeness, I could insert `indexName` into `createdIndexSet`, as I
see this was never actually done before. Thoughts @kfaraz?
--
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]