Re: [PR] Allow multiple stream supervisors to ingest into same datasource (druid)

["kfaraz (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org