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]