loserwang1024 commented on code in PR #4435:
URL: https://github.com/apache/flink-cdc/pull/4435#discussion_r4322707389
##########
flink-cdc-connect/flink-cdc-source-connectors/flink-connector-postgres-cdc/src/main/java/io/debezium/connector/postgresql/connection/PostgresConnection.java:
##########
@@ -709,8 +709,13 @@ private Optional<ColumnEditor> doReadTableColumn(
// - When querying 'user_sink', the pattern may also match 'userbsink'
(due to '_')
// - When querying 'user%data' (where % is literal), it may match
'user_test_data' (due to
// '%')
+ // Both the schema name and table name are passed to
DatabaseMetaData#getColumns as LIKE
Review Comment:
Thanks for the fix — I traced the mechanism end to end and the change is
correct. One request on how we record it, plus some context that may save you a
round trip.
Upstream Debezium already fixed this, but differently. It's DBZ-9305 "Fix
pgoutput decoder metadata query with wildcard characters", first released in
3.3.0.Final. Instead of post-filtering the result set, it escapes the LIKE
pattern at the call site
```java
final String schemaNamePattern = createPatternFromName(tableId.schema(),
databaseMetaData.getSearchStringEscape());
final String tableNamePattern = createPatternFromName(tableId.table(),
databaseMetaData.getSearchStringEscape());
try (ResultSet columnMetadata = databaseMetaData.getColumns(null,
schemaNamePattern, tableNamePattern, null)) {
```
Their test
(PostgresConnectorIT#shouldStreamSuccessfullyWithSchemasWithUnderscores) uses
schemas test_ / test1 each holding a table tab — essentially the same shape as
your sch_test / schxtest, so it's good confirmation we converged on the same
bug.
We can't take the upstream approach on our current pin, so your approach is
the right one here. Two reasons: createPatternFromName lives in debezium-core's
JdbcConnection and doesn't exist in the 1.9.8.Final we pin; and the buggy call
site (PgOutputMessageDecoder#getTableColumnsFromDatabase) is not one of the
Debezium classes we copy into this repo, so escaping there would mean pulling
in a ~700-line class. Post-filtering inside the already-copied
PostgresConnection#doReadTableColumn is the minimal option.
What I'd like changed: please record this in the class-level javadoc's
"Copied from Debezium 1.9.8-Final with the following modifications" list, the
same way the existing DBZ-5398 entry does ("Remove this comment after bumping
debezium version to 2.0.0.Final"). Right now the FLINK-38965 entry is there but
this change adds nothing, so both filters read as permanent local behaviour
rather than tracked debt that DBZ-9305 subsumes. Suggested entry
> FLINK-39844: Modified doReadTableColumn to also filter out columns
from other schemas
returned due to PostgreSQL LIKE wildcard matching on the schema
name, e.g. querying
schema 'sch_test' also matches 'schxtest'. Upstream fixed the
same issue in DBZ-9305
(Debezium 3.3.0.Final) by escaping the LIKE pattern via
JdbcConnection#createPatternFromName before calling
DatabaseMetaData#getColumns, which
we cannot backport because the call site lives in
PgOutputMessageDecoder. Remove this
and the FLINK-38965 filter after bumping debezium version to
3.3.0.Final.
One more thing worth a line in the code comment, because it confused me at
first: of the two callers of doReadTableColumn, only readColumnForDecoder is
actually protected by this filter. The other one (readTableColumn, reached from
JdbcConnection#getColumnsDetails) is passed a TableId built from the same*
result-set row (getString(2) / getString(3)), so both the existing table-name
check and your new schema check are tautologies on that path — it's covered by
tableFilter.isIncluded(tableId) instead. Noting this stops the next reader from
thinking the filter is redundant, or "simplifying" it away.
--
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]
