unikdahal commented on code in PR #5318:
URL: https://github.com/apache/datafusion-comet/pull/5318#discussion_r4113081142
##########
spark/src/test/scala/org/apache/comet/CometIcebergWriteActionSuite.scala:
##########
@@ -774,16 +774,9 @@ class CometIcebergWriteActionSuite
}
}
- test("native acceleration: ReplaceData (CoW MERGE) falls back (MergeRowsExec
not Comet)") {
- // TODO(comet-merge-rows): native MERGE engagement requires a Comet
equivalent of Iceberg's
- // `MergeRowsExec` (the per-row dispatch operator that assigns
__row_operation codes from
- // MATCHED/NOT MATCHED clauses). Without it, `MergeRowsExec` stays JVM,
the upstream chain
- // breaks Comet-native partway, and `requiresNativeChildren=true` declines
the
- // `IcebergWriteExec -> CometIcebergWriteExec` conversion. Until that
lands, MERGE
- // falls back to the JVM two-op path -- this test pins that contract.
Native `MergeRowsExec`
- // is being added in https://github.com/apache/datafusion-comet/pull/5318;
when that lands
- // this test will start failing and needs to flip to
`assertNativeWriteEngages`.
+ test("native acceleration: ReplaceData (CoW MERGE) runs MergeRows and writer
natively") {
assumeNativeAcceleration()
+ assume(isSpark35Plus && !isSpark41Plus, "native MergeRows requires Spark
3.5 through 4.0")
Review Comment:
Fixed. Spark 4.1+ now keeps the JVM fallback assertion and verifies the
native writer does not engage; 3.5/4.0 still assert the native path. Updated
the nearby comment as well.
--
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]
