peterxcli opened a new pull request, #5680:
URL: https://github.com/apache/datafusion-comet/pull/5680
## Which issue does this PR close?
Closes #5672.
## Rationale for this change
`rpad(s, len)` / `lpad(s, len)` (and the 3-arg forms with a literal pad) run
natively by default, and `spark_read_side_padding_internal` unwrapped every
value of the length array. Any NULL in the length column panicked with `called
Option::unwrap() on a None value` and failed the task with a
`CometNativeException`. Spark's `StringRPad` / `StringLPad` are null-intolerant
and return NULL for that row.
## What changes are included in this PR?
- `read_side_padding.rs`: in the array-length path (used by both the
`[Array, Array]` and `[Array, Array, Scalar]` arms) a NULL length now yields a
NULL row, like a NULL string. The output-buffer sizing pass only looks at
non-null lengths, since values under null slots are unspecified. Non-null rows
are unchanged. The scalar-length and dictionary paths only ever receive
`Int32(Some(_))` (Spark folds NULL-literal arguments to NULL before Comet sees
them) and are untouched.
- Rust unit tests: 2-arg and 3-arg `rpad`/`lpad` (and `read_side_padding`)
with a length array containing `None` → NULL for that row, other rows
unchanged; null string + null length; all-null lengths.
- `string_rpad.sql` / `string_lpad.sql`: rows `('hi', NULL, 'x')` and
`(NULL, NULL, 'x')` in the Parquet-backed table, plus a `column + column +
literal` query (`rpad(s, len, 'x')`) so the 3-arg array-length arm is covered
natively.
- `CometStringExpressionSuite`: `lpad/rpad with NULL length` builds the rows
explicitly (`FuzzDataGenerator` cannot generate NULL integers, #5389) and
checks Spark vs Comet with `checkSparkAnswerAndOperator` for the 2-arg and
3-arg forms.
## How are these changes tested?
- `cd native && cargo test -p datafusion-comet-spark-expr
read_side_padding`: 12 passed (4 new). The 4 new tests fail on `main` with the
panic from the issue.
- `cargo fmt --all -- --check` and `cargo clippy --all-targets --workspace
-- -D warnings`: clean.
- `./mvnw spotless:check test -Dtest=none
-Dsuites="org.apache.comet.CometStringExpressionSuite,org.apache.comet.CometSqlFileTestSuite
pad.sql"` (Spark 4.1.3): 2 suites, 36 tests, 0 failures, including `lpad/rpad
with NULL length`, `sql-file: expressions/string/string_lpad.sql` and
`sql-file: expressions/string/string_rpad.sql`.
--
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]
