andygrove commented on PR #5651: URL: https://github.com/apache/datafusion-comet/pull/5651#issuecomment-5531800029
> Reviewed `f13e1118` against `10537e14`. The upstream encoder is a simple fit without adding a Comet-specific path rewriter. I found no additional material defect in the dependency/path change. > > [P2] The [existing native-writer coverage concern](https://github.com/apache/datafusion-comet/pull/5651#discussion_r3919497917) remains. The new INSERT does not enable native writing, so the directory comparison can pass with JVM writers on both sides. Please use `assertNativeWriteEngages`. Its helper also enables the local-table-scan conversion needed by the VALUES input and asserts `CometIcebergWriteExec`. Showing the repaired test fail with the old pin and pass with the new pin would establish the regression coverage. > > A focused component check using the locked Rust encoder matched Java `URLEncoder` in all 160 cases. The old raw-formatting adapter differed in 89. The lock audit found only the two Iceberg records changed, with no package-version changes. These are component/source checks, not a Spark/native-writer or storage round trip. Current CI has 55 successful, 10 running and 7 skipped checks. Successful shard metadata alone does not prove this test exercised native writing. This is now fixed -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
