FrankChen021 commented on code in PR #19805:
URL: https://github.com/apache/druid/pull/19805#discussion_r3690774730


##########
server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java:
##########
@@ -239,7 +239,7 @@ protected boolean 
isRootCausePacketTooBigException(Throwable t)
   /**
    * Creates the given table and indexes if the table doesn't already exist.
    */
-  public void createTable(final String tableName, final Iterable<String> sql)
+  public void createTableIfNotExists(final String tableName, final 
Iterable<String> sql)

Review Comment:
   [P1] Preserve the renamed public connector methods
   
   This removes the public createTable(String, Iterable) method rather than 
retaining a compatibility bridge; the same patch removes public 
createIndex(...) and createEntryTable(...). Third-party metadata extensions 
compiled against an earlier Druid release can therefore fail with 
NoSuchMethodError, and subclasses overriding these methods will silently stop 
intercepting the connector's internal DDL calls. Keep deprecated delegating 
entry points—and preserve virtual dispatch for old overrides—through a 
compatibility window.



##########
server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java:
##########
@@ -305,24 +305,30 @@ public void createPendingSegmentsTable(final String 
tableName)
                 + "  sequence_prev_id VARCHAR(255) NOT NULL,\n"
                 + "  sequence_name_prev_id_sha1 VARCHAR(255) NOT NULL,\n"
                 + "  payload %2$s NOT NULL,\n"
+                + "  upgraded_from_segment_id VARCHAR(255),\n"
+                + "  task_allocator_id VARCHAR(255),\n"
                 + "  PRIMARY KEY (id),\n"
                 + "  UNIQUE (sequence_name_prev_id_sha1)\n"
                 + ")",
                 tableName, getPayloadType(), getQuoteString(), getCollation()
             )
         )
     );
-    createIndex(
+    createIndexIfNotExists(
         tableName,
         "IDX_%S_DATASOURCE_END",
         List.of("dataSource", quoteColumn("end"))
     );
-    createIndex(
+    createIndexIfNotExists(
         tableName,
         "IDX_%S_DATASOURCE_SEQUENCE",
         List.of("dataSource", "sequence_name")
     );
-    alterPendingSegmentsTable(tableName);
+    createIndexIfNotExists(

Review Comment:
   [P2] Keep migration in the named pending-table helper
   
   The public createPendingSegmentsTable(String) previously migrated existing 
tables by calling alterPendingSegmentsTable(tableName). It now tries to create 
the task-allocator index before an old table has that column, swallows the 
resulting DDL error, and returns without adding either new column. The 
no-argument wrapper still migrates, but direct callers of this public overload 
now leave pre-upgrade tables unusable by code that expects task_allocator_id 
and upgraded_from_segment_id. Restore migration within this overload, with 
index creation after the columns are ensured.



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