[PR] fix: fall back for concat_ws with array arguments instead of failing natively [datafusion-comet]

["peterxcli (via GitHub)" <gi...@apache.org>]

peterxcli opened a new pull request, #5679:
URL: https://github.com/apache/datafusion-comet/pull/5679

   ## Which issue does this PR close?
   
   Closes #5675.
   
   ## Rationale for this change
   
   Spark's `ConcatWs` accepts `array<string>` arguments after the separator and flattens their elements into the strings to join (`concat_ws(',', array('a','b'), 'c')` = `a,b,c`, null elements skipped). `CometConcatWs.getSupportLevel` never inspected child types and lowered every non-foldable call to DataFusion's `concat_ws`, which accepts only string arguments, so `concat_ws(',', arr, s)` failed at native execution with `Input was List(...) which is not a supported datatype for concat_ws function` where Spark returns a result.
   
   ## What changes are included in this PR?
   
   - `CometConcatWs` mixes in `CodegenDispatchFallback` and returns `Unsupported` (with a documented reason) whenever any argument has an `ArrayType`. These calls now run through the JVM codegen dispatcher (Spark's own `ConcatWs.doGenCode` inside the Comet pipeline) and fall back to Spark with that reason only when `spark.comet.exec.scalaUDF.codegen.enabled=false`. The NULL-separator shortcut and the native path for plain string arguments are unchanged.
   - `expressions.md`: `concat_ws` is now `Hybrid` (what `GenerateDocs` emits for a `CodegenDispatchFallback` serde) with a note; the compatibility page picks up the new unsupported reason automatically.
   - Follow-up (not in this PR): a Spark-compatible native `concat_ws` kernel that flattens list arguments (skipping null elements like Spark) so these calls can leave the dispatcher.
   
   ## How are these changes tested?
   
   - New `CometStringExpressionSuite` test `concat_ws with array<string> arguments`: over a Parquet table with an `array<string>` column it runs `concat_ws(',', arr, s)`, `concat_ws(',', s, arr)`, `concat_ws(',', arr)`, `concat_ws('-', arr, s, arr)`, `concat_ws(',', split(s, ' '))` and `concat_ws(',', split(s, ' '), arr)` and asserts Spark's answer with the whole plan in Comet and the dispatcher actually running; with the dispatcher disabled it asserts the Spark answer plus the serde's fallback reason; it also asserts a NULL separator still runs natively and that plain string arguments keep the native path.
   - `sql-tests/expressions/string/concat_ws.sql`: a new `array<string>` table and eight query blocks (column, mixed, single, repeated, `split`-produced and literal array arguments, NULL separator), all asserting Spark-identical results.
   - Ran `CometStringExpressionSuite` and `CometSqlFileTestSuite concat_ws` (Spark 4.1 profile): 35/35 passed. Native code is untouched.
   


-- 
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-unsubscribe@datafusion.apache.org

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


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

Re: [PR] fix: fall back for concat_ws with array arguments instead of failing natively [datafusion-comet]

["peterxcli (via GitHub)" <gi...@apache.org>]

peterxcli commented on PR #5679:
URL: https://github.com/apache/datafusion-comet/pull/5679#issuecomment-5544002921

   > Maybe we should also implement native array support as separate follow-up items.
   
   filed https://github.com/apache/datafusion-comet/issues/5687, thanks for the suggestion!


-- 
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-unsubscribe@datafusion.apache.org

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


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

Re: [PR] fix: fall back for concat_ws with array arguments instead of failing natively [datafusion-comet]

["sunchao (via GitHub)" <gi...@apache.org>]

sunchao commented on PR #5679:
URL: https://github.com/apache/datafusion-comet/pull/5679#issuecomment-5543800985

   Merged, thanks @peterxcli !
   
   Maybe we should also implement native array support as separate follow-up items.


-- 
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-unsubscribe@datafusion.apache.org

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


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

Re: [PR] fix: fall back for concat_ws with array arguments instead of failing natively [datafusion-comet]

["sunchao (via GitHub)" <gi...@apache.org>]

sunchao merged PR #5679:
URL: https://github.com/apache/datafusion-comet/pull/5679


-- 
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-unsubscribe@datafusion.apache.org

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


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