SEZ9 commented on PR #12015:
URL: https://github.com/apache/seatunnel/pull/12015#issuecomment-5673565372
Thanks for re-posting, @luozihen — the tail of the previous message was
indeed cut off.
**F4** — Thanks for laying out the ordering:
`TablePlaceholderProcessor.replaceTablePlaceholder` running first but only
touching top-level String / single-element-String-List values, leaving the
nested `multi_table_config` map for `expandPrimaryKeyPlaceholder`, and failing
fast with `JDBC-12` when `${primary_key}` is used against a table without a
primary key. That is the behavior I was hoping for. Could you point me to the
changed lines in this PR that implement the fail-fast path (and any test that
exercises it) so I can confirm against the diff? It would also help to state
the fail-fast semantics in the option docs so users know what to expect when a
matched table has no PK.
**F6** — Compiling every pattern up front in `toCompiledPatternMap` via
`compilePattern` and surfacing `JDBC-12` immediately is what I was after.
Please point me to `testResolveMultiTablePrimaryKeysInvalidRegexFails` and
`testCompilePatternInvalidFails` in the diff and I will mark this resolved.
**F7** — Good to hear
`ReadableConfigTest#testToConfigPreservesDottedKeyExpansion` is in place; I
will verify it in the changed test file. Your message was truncated again at
"noted as a follow-up in the P" — could you confirm where the deferred
engine-level IT/E2E is tracked (PR description or a tracking issue) and paste
the reference here? I am fine deferring the E2E as long as it is explicitly
tracked.
Still open from the previous review: F1, F2, F3, F5 and F8. If any of those
were also in the truncated part of your reply, please re-post them the same way.
<!-- streview-comment:1045 -->
--
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]
