Re: [PR] fix: replace() returns wrong result for empty-string search [datafusion-comet]

2026-02-06 Thread via GitHub 


andygrove closed pull request #3391: fix: replace() returns wrong result for 
empty-string search
URL: https://github.com/apache/datafusion-comet/pull/3391


-- 
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]

Re: [PR] fix: replace() returns wrong result for empty-string search [datafusion-comet]

2026-02-06 Thread via GitHub 


andygrove commented on PR #3391:
URL: 
https://github.com/apache/datafusion-comet/pull/3391#issuecomment-3863053796

   Thanks for looking into this @wattt3. You are correct - the original issue 
was invalid. Sorry about that.


-- 
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]

Re: [PR] fix: replace() returns wrong result for empty-string search [datafusion-comet]

2026-02-05 Thread via GitHub 


wattt3 commented on PR #3391:
URL: 
https://github.com/apache/datafusion-comet/pull/3391#issuecomment-3853849180

   @andygrove 
   Due to environment issues, I couldn't run the test locally at the time.
   I changed to a SQL file-based approach, but the result differs from the 
issue description.
   https://github.com/user-attachments/assets/f870f55d-4376-44df-98f1-f7d042fc9d35";
 />


-- 
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]

Re: [PR] fix: replace() returns wrong result for empty-string search [datafusion-comet]

2026-02-04 Thread via GitHub 


andygrove commented on PR #3391:
URL: 
https://github.com/apache/datafusion-comet/pull/3391#issuecomment-3848515054

   There is a test failure: 
   
   ```
   2026-02-04T16:17:16.3295718Z - replace with empty search string *** FAILED 
*** (166 milliseconds)
   2026-02-04T16:17:16.3296584Z   Results do not match for query:
   2026-02-04T16:17:16.3299606Z   Timezone: 
sun.util.calendar.ZoneInfo[id="America/Los_Angeles",offset=-2880,dstSavings=360,useDaylight=true,transitions=311,lastRule=java.util.SimpleTimeZone[id=America/Los_Angeles,offset=-2880,dstSavings=360,useDaylight=true,startYear=0,startMode=3,startMonth=2,startDay=8,startDayOfWeek=1,startTime=720,startTimeMode=0,endMode=3,endMonth=10,endDay=1,endDayOfWeek=1,endTime=720,endTimeMode=0]]
   2026-02-04T16:17:16.3302580Z   Timezone Env: 
   2026-02-04T16:17:16.3302956Z   
   2026-02-04T16:17:16.3303402Z   == Parsed Logical Plan ==
   2026-02-04T16:17:16.3304730Z   Project [replace(col#89982, , x) AS 
replace(col, , x)#89993, replace(col#89982, , ) AS replace(col, , )#89994]
   2026-02-04T16:17:16.3306566Z   +- SubqueryAlias spark_catalog.default.test
   2026-02-04T16:17:16.3307192Z  +- Relation 
spark_catalog.default.test[col#89982] parquet
   2026-02-04T16:17:16.3307674Z   
   2026-02-04T16:17:16.3307942Z   == Analyzed Logical Plan ==
   2026-02-04T16:17:16.3308425Z   replace(col, , x): string, replace(col, , ): 
string
   2026-02-04T16:17:16.3309283Z   Project [replace(col#89982, , x) AS 
replace(col, , x)#89993, replace(col#89982, , ) AS replace(col, , )#89994]
   2026-02-04T16:17:16.3310070Z   +- SubqueryAlias spark_catalog.default.test
   2026-02-04T16:17:16.3310839Z  +- Relation 
spark_catalog.default.test[col#89982] parquet
   2026-02-04T16:17:16.3311330Z   
   2026-02-04T16:17:16.3311631Z   == Optimized Logical Plan ==
   2026-02-04T16:17:16.3312688Z   Project [replace(col#89982, , x) AS 
replace(col, , x)#89993, replace(col#89982, , ) AS replace(col, , )#89994]
   2026-02-04T16:17:16.3313560Z   +- Relation 
spark_catalog.default.test[col#89982] parquet
   2026-02-04T16:17:16.3314032Z   
   2026-02-04T16:17:16.3314277Z   == Physical Plan ==
   2026-02-04T16:17:16.3314578Z   *(1) CometColumnarToRow
   2026-02-04T16:17:16.3315628Z   +- CometProject [replace(col, , x)#89993, 
replace(col, , )#89994], [replace(col#89982, , x) AS replace(col, , x)#89993, 
replace(col#89982, , ) AS replace(col, , )#89994]
   2026-02-04T16:17:16.3318449Z  +- CometScan [native_iceberg_compat] 
parquet spark_catalog.default.test[col#89982] Batched: true, DataFilters: [], 
Format: CometParquet, Location: InMemoryFileIndex(1 
paths)[file:/__w/datafusion-comet/datafusion-comet/spark/spark-warehouse/test], 
PartitionFilters: [], PushedFilters: [], ReadSchema: struct
   2026-02-04T16:17:16.3320372Z   
   2026-02-04T16:17:16.3320776Z   == Results ==
   2026-02-04T16:17:16.3321058Z   
   2026-02-04T16:17:16.3321294Z   == Results ==
   2026-02-04T16:17:16.3321798Z   !== Correct Answer - 3 == 
  == Spark Answer - 3 ==
   2026-02-04T16:17:16.3322827Zstruct   struct
   2026-02-04T16:17:16.3323753Z   ![,]  
  [null,null]
   2026-02-04T16:17:16.3324349Z   ![hello,hello]
  [x,]
   2026-02-04T16:17:16.3325067Z   ![null,null]  
  [xhxexlxlxox,hello] (QueryTest.scala:244)
   ```


-- 
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]

Re: [PR] fix: replace() returns wrong result for empty-string search [datafusion-comet]

2026-02-04 Thread via GitHub 


andygrove commented on PR #3391:
URL: 
https://github.com/apache/datafusion-comet/pull/3391#issuecomment-3848281938

   Thanks @wattt3. LGTM. I left a comment about moving the tests to the new sql 
file based approach. I will be happy to approve the PR once that is done.


-- 
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]

Re: [PR] fix: replace() returns wrong result for empty-string search [datafusion-comet]

2026-02-04 Thread via GitHub 


andygrove commented on code in PR #3391:
URL: https://github.com/apache/datafusion-comet/pull/3391#discussion_r2764717973


##
spark/src/test/scala/org/apache/comet/CometStringExpressionSuite.scala:
##
@@ -280,6 +280,16 @@ class CometStringExpressionSuite extends CometTestBase {
 }
   }
 
+  test("replace with empty search string") {

Review Comment:
   Could you add these tests in the new sql file testing approach instead?
   
   https://datafusion.apache.org/comet/contributor-guide/sql-file-tests.html



-- 
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]