DanielLeens commented on PR #12327: URL: https://github.com/apache/seatunnel/pull/12327#issuecomment-5689995868
Thanks for the very thorough follow-up, @Dev-next-gen -- really appreciate you double-checking both points instead of just taking my review at face value. On the ClickHouse test-coverage note: you're right, and it's a good catch on my part being imprecise. I just re-checked -- `ClickhouseCatalogUtil` extends `connector-common`'s `CatalogUtil`, and nothing else in `connector-clickhouse` references its own `CreateTableParser` copy anywhere in the module. So that copy is effectively dead code today, not a second live path that needs its own test -- `CreateTableParserTest` already covers the parser ClickHouse actually uses at runtime. MaxCompute is the one with a genuinely untested live copy, as you said. My original comment conflated "duplicated file" with "duplicated live code path" -- thanks for correcting that. On the CI: your job-by-job breakdown lines up with what I'd expect. `OpengaussCDCIT`, `RocketMqIT`, and `PaimonWithS3IT` failing the same way on `dev` (unrelated to your branch), the windows `unit-test` / `connector-file-sftp-it` failures being Maven-repo network issues before compilation even started, and `kudu-connector-it` being cancelled -- none of those touch this change. The one job that does live in a module you touched, `ClickhouseIT.clickhouseWithCreateSchemaWhenNotExist` on JDK 8, and you've already shown the generated DDL is byte-identical with and without your fix, and that the failure is a row-count/table-cleanup issue between containers rather than a parsing difference -- that's solid evidence it's a pre-existing test-isolation flake, not something your diff introduced. You don't need to chase the "why 99 rows" question further for this PR; it isn't something your change caused. My APPROVED review from earlier still stands -- nothing here changes that assessment. Thanks again for the careful, evidence-based work, and welcome to the community! -- 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]
