Copilot commented on code in PR #4473:
URL: https://github.com/apache/flink-cdc/pull/4473#discussion_r3612370995
##########
flink-cdc-runtime/src/test/java/org/apache/flink/cdc/common/route/TableIdRouterTest.java:
##########
@@ -177,6 +177,60 @@ private static List<String> testStdRegExpRoute(
.collect(Collectors.toList());
}
+ @Test
+ void testRouteWithoutBackReferenceWithMultiDigitSuffix() {
+ // Regression test: when the source-table regex matches multi-digit
suffixes (e.g. 10-16)
+ // but the sink-table does NOT contain a `$1` back-reference, every
matched source table
+ // should still be routed to the exact sink-table declared in the
routing rule. Previously
+ // TableIdRouter.resolveReplacement used Matcher.find(), which only
matched a prefix of
+ // the source table ID, leaving the unmatched tail characters to be
appended to the
+ // sink-table after replaceAll. As a result, table `db_6.table_13` was
wrongly routed to
+ // `db_6.table_3`, `db_6.table_14` to `db_6.table_4`, etc.
Review Comment:
The example routing output in this comment is incorrect (it drops the
configured sink-table prefix). In this regression scenario, the buggy behavior
was appending the leftover digit to the configured sink-table (e.g.
`new_db_6.table_merged3`), not routing to `db_6.table_3`. Updating the comment
will prevent future confusion when diagnosing regressions.
##########
flink-cdc-common/src/main/java/org/apache/flink/cdc/common/route/TableIdRouter.java:
##########
@@ -173,9 +173,20 @@ private TableId resolveReplacement(
if (route.f2 != null) {
return TableId.parse(route.f1.replace(route.f2,
originalTable.getTableName()));
} else {
+ // Use matches() so that the entire source table id is consumed.
After matches() the
+ // matcher holds the full match, and we feed it through
appendReplacement /
+ // appendTail to perform the `$N` substitution only on the matched
region. We cannot
+ // use Matcher.replaceAll here because it internally calls reset()
+ find(), which
+ // would only match a prefix of the source table id (when the
regex alternation
+ // order matters, e.g. `([1-9]|1[0-6])` matches `1` from `13` and
leaves `3` as a
+ // leftover that gets appended to the sink table name, routing
+ // `table_13` to `table_3`).
Review Comment:
This comment’s example (`table_13` → `table_3`) is ambiguous and doesn’t
match the actual failure mode described in the PR (the leftover characters are
appended to the configured sink-table name, e.g. `...merged3`). Consider
rephrasing to reflect the real output so readers can connect the rationale to
the observed bug.
--
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]
