sunchao commented on code in PR #7123:
URL: https://github.com/apache/datafusion-comet/pull/7123#discussion_r4321155083
##########
spark/src/main/scala/org/apache/comet/CometConf.scala:
##########
@@ -1275,9 +1275,12 @@ object CometConf extends ShimCometConf {
"the broadcast threshold), so the row to Arrow copy is usually cheap -
though an " +
"explicit BROADCAST hint or a selective filter above the scan can make
it larger. With " +
"dynamic partition pruning the dimension is also read a second time
for the pruning " +
- "subquery. Set this to false to keep such build sides on Spark.")
+ "subquery. This is off by default because Arrow strings must be valid
UTF-8, so " +
+ "Comet decodes invalid UTF-8 bytes (for example in a Latin-1 Text
file) to U+FFFD, " +
+ "which can change results that depend on the original bytes. See the
compatibility " +
+ "guide.")
.booleanConf
- .createWithDefault(true)
+ .createWithDefault(false)
Review Comment:
[P2] Enable the remaining bridge-dependent tests explicitly. The Text
nested-loop join test and nested native-join test in `CometExecSuite`
(registrations at lines 616 and 806) still use plain `test`. With this new
default, their Text scans correctly remain on Spark, but their assertions
require a bridge and native join. Both fail on Spark 4.1.3/JDK 21 at lines 633
and 835. Explicitly enabling the configuration makes both pass. This breaks the
CI `exec` suite. Please register these tests through `bridgeTest` while keeping
the production default off.
Evidence: Reproduced with `./mvnw test -Dtest=none
'-Dsuites=org.apache.comet.exec.CometExecSuite @broadcast nested loop join
build side with a text source goes native
(#6008),org.apache.comet.exec.CometExecSuite @a native broadcast join nested in
a Spark-staying join keeps its bridge (#6008)'` using the local build settings
described above. Default configuration: 0 passed, 2 failed, with `expected the
text build side to be bridged` and `the inner join should be a
CometBroadcastHashJoinExec`. Repeating the same selection with
`JAVA_TOOL_OPTIONS=-Dspark.comet.convert.broadcastBuildSide.enabled=true`
produced 2 passed, 0 failed.
--
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]
