jordepic commented on PR #4251: URL: https://github.com/apache/datafusion-comet/pull/4251#issuecomment-4401024774
> On the Spark 3.4 CI failure from before the force-push, do you still have the stack? I checked Iceberg 1.5.2 and the SparkScan / SparkPartitioningAwareScan hierarchy matches 3.5, so the three accessors in `IcebergReflection` should resolve the same way. My guess is the failure is in the test's own fluent-builder reflection against the Actions API rather than the new reflection helpers, but hard to say without the trace. in Iceberg 1.5.2 (used by the spark-3.4 profile), option(String, String) on the rewrite action chain is declared on the package-private BaseSparkAction class. My invokeMethod calls getMethod(...).invoke(...) directly. Since JDK 11+, you can't invoke a method whose declaring class is non-public from outside its package — even if the method itself is public — without setAccessible(true). In Iceberg 1.8.1+ (spark-3.5/4.x) the same method is declared on a public class, so getMethod(...).invoke(...) worked transparently. > Smaller thing: the metadata-table guard at CometScanRule.scala:98-116 already rejects .position_deletes via suffix match, which I think is what keeps the new SparkStagedScan case safely scoped to data-file rewrites and away from RewritePositionDeleteFilesSparkAction. A short comment near the StagedScan match pointing at that upstream check would have saved me a detour on review. I wish I could say that I actually remembered to check this and maliciously omitted a comment to make your life harder, but the truth is that I was just negligent. Great catch! I'll add a comment, thanks Matt! In response to your comment, I now have 5 tests. 1) Bin packing 2) Sorting 3) Z ordering 4) Bin packing with positional and equality deletes 5) Ensuring that we do not convert `RewritePositionDeleteFilesSparkAction` -- 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]
