parthchandra commented on PR #1901:
URL: 
https://github.com/apache/datafusion-comet/pull/1901#issuecomment-3006474262

   I don't think this PR is making the correct change. 
   With this PR the removed test fails to execute the query (let alone pass the 
assertion)
   ```
     test("SortMergeJoin with unsupported key type should fall back to Spark") {
       withSQLConf(
         SQLConf.SESSION_LOCAL_TIMEZONE.key -> "Asia/Kathmandu",
         SQLConf.ADAPTIVE_AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1",
         SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
         withTable("t1", "t2") {
           sql("CREATE TABLE t1(name STRING, time TIMESTAMP) USING PARQUET")
           sql("INSERT OVERWRITE t1 VALUES('a', timestamp'2019-01-01 
11:11:11')")
   
           sql("CREATE TABLE t2(name STRING, time TIMESTAMP) USING PARQUET")
           sql("INSERT OVERWRITE t2 VALUES('a', timestamp'2019-01-01 
11:11:11')")
   
           val df = sql("SELECT * FROM t1 JOIN t2 ON t1.time = t2.time")
           val (sparkPlan, cometPlan) = checkSparkAnswer(df)                // 
should NOT fail here, but does
           assert(sparkPlan.canonicalized === cometPlan.canonicalized)       // 
should fail here
         }
       }
     }
   
   ```
   The reason for the failure is - 
   ```
   org.apache.comet.CometNativeException: Unsupported data type in sort merge 
join comparator: Timestamp(Microsecond, Some("UTC"))
   ```
   This means that `supportedSortMergeJoinEqualType` should not return true for 
`Timestamp`
   
   The newly added test does not exercise the change. The plan (see below) does 
not include a SMJ and the test passes with and without the changes in PR. 
   ```
   AdaptiveSparkPlan isFinalPlan=false
   +- BroadcastHashJoin [ts#4], [ts#10], Inner, BuildRight, false
      :- LocalTableScan [ts#4]
      +- BroadcastExchange HashedRelationBroadcastMode(List(input[0, timestamp, 
true]),false), [plan_id=106]
         +- LocalTableScan [ts#10]
   ```


-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to