wgzhao commented on PR #12373:
URL: https://github.com/apache/seatunnel/pull/12373#issuecomment-5723357748
Thanks - the trace was right, and it was the reason for this push. Rather
than taking Option A or B as written, I removed the coupling itself, because
the literals were not needed by this case at all.
**What was wrong.** The fixture spelled the columns as
`set('REAL_AS_FLOAT','PIPES_AS_CONCAT','NO_UNSIGNED_SUBTRACTION')` and
`enum('unsigned','signed')`. Both contain the substring the catalog still
searches for when deriving `UNSIGNED`, so the snapshot produced `SET UNSIGNED`
/ `ENUM UNSIGNED` and the run died at catalog discovery with `COMMON-21` -
#12333's defect, reached before any of the DDL this PR exists to exercise could
run. Your reading of the job log matches what the source does here.
**What changed** (`5fd406c88`):
- The option lists are now neutral:
`set('REAL_AS_FLOAT','PIPES_AS_CONCAT')`, `enum('signed','sealed')`, and the
widened `SET` adds `ANSI_QUOTES` instead of the `NO_UNSIGNED_SUBTRACTION`
literal.
- No literal containing `unsigned` remains in the PR's SQL - the only
occurrences left are comments explaining why it is avoided.
- The PR description now records this explicitly in place of the
single-dependency claim, including the `COMMON-21` evidence from the first run.
**Why this over the two options you listed.** The trigger for #12355 is the
option list surviving the rebuild, not the word `unsigned`, so the literal was
incidental to this case while creating a hard dependency on an unrelated,
unmerged fix. With it gone, the only dependency is #12355, and coverage of the
literal-with-`unsigned` shape stays where it belongs: the catalog side in
`connector-jdbc`, where #12333 adds it. This reaches the same end state as
Option A (green only when its dependency is in) without keeping an accidental
cross-PR coupling in the fixture.
The next run should now fail (or converge-time-out) at the `ALTER TABLE`
this case is about rather than at catalog discovery. If you would rather keep
the literal so that this fixture also covers #12333's path, say so and I will
restore it and hold the PR for both merges instead.
One thing worth flagging while you are reviewing these: your approvals on
#12333 and #12355 are recorded, but both PRs still report
`REVIEW_REQUIRED`/`BLOCKED`. The `dev` ruleset requires an approving review
from someone with write access, and contributor approvals are not counted
towards that count - so those two still need a committer to approve (and their
`Build` check to come back green) before they can move.
--
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]
