imply-cheddar commented on code in PR #14470:
URL: https://github.com/apache/druid/pull/14470#discussion_r1239247521
##########
server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java:
##########
@@ -830,4 +845,73 @@ public Void withHandle(Handle handle)
log.warn(e, "Exception while deleting records from table");
}
}
+
+ /**
+ * create index on the table if not already exist with retry.
+ *
+ * @param handle Connection handle
+ * @param table name of the table to fetch the index map
+ * @return Map of the uppercase index name to List of the columns in index
+ */
+ public Map<String, List<String>> getIndexOnTable(Handle handle, String table)
+ {
+ Map<String, List<String>> res = new HashMap<>();
+ try {
+ DatabaseMetaData databaseMetaData = handle.getConnection().getMetaData();
+ ResultSet resultSet = databaseMetaData.getIndexInfo(null, null,
table.toUpperCase(Locale.ENGLISH), false, false);
+
+ while (resultSet.next()) {
+ String indexName = resultSet.getString("INDEX_NAME");
+ String columnName = resultSet.getString("COLUMN_NAME");
+ if (org.apache.commons.lang.StringUtils.isNotBlank(indexName) &&
org.apache.commons.lang.StringUtils.isNotBlank(
+ columnName)) {
+ columnName = StringUtils.toUpperCase(columnName);
+ List<String> cols = res.getOrDefault(indexName, new ArrayList<>());
+ cols.add(columnName);
+ res.put(indexName, cols);
+ }
+ }
+ }
+ catch (Exception e) {
+ log.error(e, "Exception while listing the index on table %s ", table);
+ }
+ return res;
+ }
+
+ /**
+ * create index on the table if not already exist with retry.
+ *
+ * @param tableName Name of the table to create index on
+ * @param checkString case-insensitive string which substring of index name,
it helps to check the existing index on table
+ * @param indexSQL SQL statement to create index
+ */
+ public void createIndex(final String tableName, final String checkString,
final String indexSQL)
+ {
+ try {
+ retryWithHandle(
+ new HandleCallback<Void>()
+ {
+ @Override
+ public Void withHandle(Handle handle)
+ {
+ if (tableExists(handle, tableName)) {
+ Map<String, List<String>> indexMap = getIndexOnTable(handle,
tableName);
Review Comment:
Your function signatures are going to cause us to read all of the indexes
for all of the tables on every single check. Not a big deal for 2, but better
to be more conscious of the number of DB calls that you are making.
If you adjust the signature of `createIndex` to be a bit more developer
friendly (as suggested above), then I think that you can build the exact index
name that you expect and check for only that. Alternatively, you should read
the set of indexes once and then reuse that map for each of these calls.
##########
server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java:
##########
@@ -830,4 +845,73 @@ public Void withHandle(Handle handle)
log.warn(e, "Exception while deleting records from table");
}
}
+
+ /**
+ * create index on the table if not already exist with retry.
+ *
+ * @param handle Connection handle
+ * @param table name of the table to fetch the index map
+ * @return Map of the uppercase index name to List of the columns in index
+ */
+ public Map<String, List<String>> getIndexOnTable(Handle handle, String table)
+ {
+ Map<String, List<String>> res = new HashMap<>();
+ try {
+ DatabaseMetaData databaseMetaData = handle.getConnection().getMetaData();
+ ResultSet resultSet = databaseMetaData.getIndexInfo(null, null,
table.toUpperCase(Locale.ENGLISH), false, false);
+
+ while (resultSet.next()) {
+ String indexName = resultSet.getString("INDEX_NAME");
+ String columnName = resultSet.getString("COLUMN_NAME");
+ if (org.apache.commons.lang.StringUtils.isNotBlank(indexName) &&
org.apache.commons.lang.StringUtils.isNotBlank(
+ columnName)) {
+ columnName = StringUtils.toUpperCase(columnName);
+ List<String> cols = res.getOrDefault(indexName, new ArrayList<>());
+ cols.add(columnName);
+ res.put(indexName, cols);
+ }
+ }
+ }
+ catch (Exception e) {
+ log.error(e, "Exception while listing the index on table %s ", table);
+ }
+ return res;
+ }
+
+ /**
+ * create index on the table if not already exist with retry.
+ *
+ * @param tableName Name of the table to create index on
+ * @param checkString case-insensitive string which substring of index name,
it helps to check the existing index on table
+ * @param indexSQL SQL statement to create index
+ */
+ public void createIndex(final String tableName, final String checkString,
final String indexSQL)
Review Comment:
This function signature is a bit error-prone for the end user. It's pretty
easy (as noted from the earlier comment on this very PR) to accidentally get
the "checkString" and the "indexSQL" wrong. It should be safe to separate out
the different parts of the create index command. You can make this signature
```
createIndex(String tableName, String indexName, List<String> indexCols)
```
And then you should be able to build the correct SQL for that inside of this
method as well as know the exact name of the index that you are trying to
create (for use in checking if it already exists)
--
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]
