SEZ9 commented on PR #10959: URL: https://github.com/apache/seatunnel/pull/10959#issuecomment-5230720952
Thanks @DanielLeens for the detailed trace — that's exactly the context I needed. You're right on both counts: 1. **On the API change**: Since `DamengCreateTableSqlBuilder` is an internal implementation class (not SPI or a documented extension point) with a single in-module caller (`DamengCatalog.getCreateTableSqls(...)`) that's updated in the same diff, the `String -> List<String>` return-type change doesn't pose a real backward-compatibility risk. The fact that the identical change already landed on `dev` via #10934 without fallout confirms this. I'm withdrawing that as a High-severity blocker — it's a non-issue in practice. 2. **On the supersession**: Agreed. With #10934 merged carrying the equivalent fix plus the comment-escaping and expanded regression tests, keeping this PR open would just leave duplicate logic on the same code path. Closing this as superseded by #10934 is the right call — no further iteration needed here. @happybrant thank you again for surfacing this issue and driving the fix forward — the problem you identified was real and is now resolved on `dev`. This isn't a knock on the work here at all; the timing just meant two fixes converged on the same spot. We'd love to see more contributions from you, and the Slack #contributors channel is a great place to sync before starting on a fix to avoid overlap like this. Closing as superseded by #10934. <!-- streview-comment:104 --> -- 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]
