kfaraz commented on code in PR #18082:
URL: https://github.com/apache/druid/pull/18082#discussion_r2137249022


##########
server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java:
##########
@@ -313,17 +313,27 @@ tableName, getPayloadType(), getQuoteString(), 
getCollation()
 
   public void createDataSourceTable(final String tableName)
   {
+    final List<String> columns = new ArrayList<>();

Review Comment:
   Couple of notes here:
   - There should be an ALTER path for existing clusters where this metadata 
table already exists.
     -  It should DROP the old index and CREATE a new one.
     - `supervisor_id` will be null for old entries. The PRIMARY KEY will fail 
for this case.
     - Consider using a composite PRIMARY KEY on (datasource + supervisor_id)
   - Having a composite PRIMARY KEY will also help while searching only by 
datasource column.
   - The old style of writing the CREATE statement was much cleaner and easy to 
read.
   Let's stick to that style since all the columns (except one) seem to be the 
same as before.



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