lvyanquan commented on PR #4474: URL: https://github.com/apache/flink-cdc/pull/4474#issuecomment-4993513284
Thanks for the work here. The changes split into three buckets with very different compatibility impact. I'd suggest applying buckets 1–2 unconditionally and gating bucket 3 behind an explicit option. **1. New operators (currently unparseable) — keep always-on** SQL: `IS [NOT] DISTINCT FROM`, `IS [NOT] UNKNOWN`, `SIMILAR TO` / `NOT SIMILAR TO`, `LIKE ... ESCAPE '<char>'`. These fail validation on the current release, so no existing pipeline can depend on them. Safe to enable unconditionally. **2. Crash → 3-valued logic, non-NULL behavior unchanged — keep always-on** SQL: `AND`, `OR`, `NOT`, `IS [NOT] TRUE`, `IS [NOT] FALSE` where the operand is a NULL boolean (nullable `BOOLEAN` column or `CAST(NULL AS BOOLEAN)`). These previously threw an NPE on NULL; results for non-NULL operands are identical. Pure crash fix, safe to enable unconditionally. **3. Silent result changes — must be gated behind an explicit option** SQL: - Comparison / set predicates: `=`, `<>`, `>`, `>=`, `<`, `<=`, `IN`, `NOT IN`, `BETWEEN`, `NOT BETWEEN`. On a NULL operand the result changes from `false`/`true` to `NULL`. For negated forms (`<>`, `NOT IN`, `NOT BETWEEN`) this is `true → NULL`, which **drops rows in `WHERE`** and **flips `CASE` / `IF` branches**. - `LIKE` / `NOT LIKE`: pattern engine changes from Java regex (`Pattern.find()`, substring match) to SQL wildcards (`%`, `_`, whole-string match). This changes results for **every row, not just NULL** — e.g. `'xabcy' LIKE 'abc'` goes `true → false`, `col LIKE 'A%'` goes `false → true`. This is a backward-incompatible change to user-facing semantics, and the silent nature makes it dangerous: after the upgrade the YAML is unchanged, the job stays healthy, and no error or warning is emitted, yet the output data changes. For a CDC sync, altered filter results change which `INSERT` / `UPDATE` / `DELETE` events reach the sink, so the sink silently diverges from the source with no signal to detect or repair it. It therefore must not be the default on a minor/patch release. **Recommendation** Introduce `pipeline.transform.expression.semantics` (enum `LEGACY` | `FLINK_SQL`, default `LEGACY`). Keep the current behavior under `LEGACY`; apply bucket 3 only under `FLINK_SQL`. Buckets 1 and 2 remain always-on. Also, to ease review, consider splitting this so buckets 1–2 and bucket 3 land as separate commits (or separate PRs) — the crash/parse fixes can then be merged quickly, while the semantic change is reviewed on its own. -- 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]
