SEZ9 commented on PR #10959: URL: https://github.com/apache/seatunnel/pull/10959#issuecomment-5225134760
Thanks @DanielLeens for the thorough trace — that's exactly the context I was missing. You're right on both counts: - **On the API change (Issue 1):** Since `DamengCreateTableSqlBuilder` is an internal catalog implementation class with a single in-module caller (`DamengCatalog.getCreateTableSqls(...)`) updated in the same diff, and it's not an SPI or documented extension point, the `String -> List<String>` change doesn't carry the downstream compatibility risk I flagged. The fact that the identical change already landed on `dev` via #10934 without fallout confirms that. I'm withdrawing this as a blocker. - **On the PR itself:** Given #10934 already merged an equivalent and more complete fix (including the comment-escaping that was still outstanding here), there's no value in keeping two implementations of the same code path. Closing this as superseded is the right call. @happybrant — thank you again for surfacing this Dameng auto-create-table issue and working on the fix. Your PR helped drive this to resolution even though #10934 landed first; that's a real contribution to the project. No further action needed here, and we'd genuinely love to see more PRs from you. **Remaining ask:** let's close #10959 as superseded by #10934. @happybrant, feel free to close it yourself, or a committer can do so — either works. <!-- streview-comment:83 --> -- 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]
