Repository navigation
[pull] main from apache:main - #272
Merged
Merged
Conversation
## Which issue does this PR close? No issue, just responding to discussion in #22723 ## Rationale for this change Agreed this was an overly strict approach. ## What changes are included in this PR? Reverting previous work ## Are these changes tested? By definition ## Are there any user-facing changes? Contributors won't be held to higher standards in SLTs than in the contributor guide.
## Which issue does this PR close? N/A ## Rationale for this change No backtrace is added when using the assert macros, so fixing that ## What changes are included in this PR? used `internal_datafusion_err` macro in the `assert_*` helpers ## Are these changes tested? yes ## Are there any user-facing changes? now have backtrace when feature enabled
…ed_append (#21794) ## Which issue does this PR close? - Closes #21568. ## Rationale for this change `ByteViewGroupValueBuilder::vectorized_append` was doing unnecessary work for short strings (≤12 bytes): for each row it called `array.value(row)` to decode the u128 view into a `&[u8]`, then called `make_view` to re-encode it back into a u128. The input `GenericByteViewArray` already stores inline values in exactly that u128 format, so the round-trip is redundant. This mirrors the existing `HAS_BUFFERS` specialisation in `vectorized_equal_to_inner`, which uses the same `data_buffers().is_empty()` guard to take a direct-view-compare fast path for inline strings. ## What changes are included in this PR? In `vectorized_append_inner`, the `Nulls::None` branch now dispatches on `arr.data_buffers().is_empty()`: - **Fast path** (no data buffers → all values ≤12 bytes inline): copies u128 views directly via `self.views.extend(rows.iter().map(|&row| arr.views()[row]))`. Arrow's validity invariant guarantees inline views are zero-padded, so direct copy is semantically identical to `value() → make_view()`. - **Slow path** (array has non-inline strings): adds `self.views.reserve(rows.len())` before the existing loop to avoid repeated reallocation. ## Are these changes tested? Covered by the existing 6 unit tests in `bytes_view::tests`, all passing unchanged. `test_byte_view_vectorized_operation_special_case` exercises the fast path directly (11-byte strings, no data buffers). ## Are there any user-facing changes? No. Internal performance improvement only. ## Benchmark `inline_null_0.0_size_1000/vectorized_append` (8-byte strings, no nulls, 1 000 rows): | | time | |---|---| | Before | 3.37 µs | | After | 495 ns | | Change | **−85.3% (6.8× faster)** |
## Which issue does this PR close? - N/A ## Rationale for this change Various improvements and cleanups to the `SELECT` reference page in the documentation. Update the documentation to cover more of the supported SQL syntax, to include more details when appropriate, and to add more examples. ## What changes are included in this PR? * Improve the `SELECT` reference page in the documentation * Update `pipe_operator.slt` to reflect that the pipe operator syntax is now supported in the default SQL dialect ## Are these changes tested? Manually and LLM-checked the examples. ## Are there any user-facing changes? Documentation improvements, but should be no regressions. No functional changes in DataFusion.
## Which issue does this PR close? - Closes #22599. ## Rationale for this change Implements the Spark `weekday` function as part of the datafusion-spark function library (#15914). Spark's `weekday` returns the day-of-week as a 0-indexed integer with Monday = 0 .. Sunday = 6, which differs from the existing `dayofweek` (1-indexed, Sunday = 1). This fills a gap for Spark-compatibility consumers. ## What changes are included in this PR? - New `SparkWeekDay` scalar UDF in `datafusion/spark/src/function/datetime/weekday.rs`, modeled on the sibling `monthname` function. - Accepts Date and (implicitly coerced) Timestamp input; returns `Int32`. - Uses Arrow's `DatePart::DayOfWeekMonday0` kernel, which is exactly 0=Monday .. 6=Sunday — a direct match for Spark semantics. - Null input propagates to null output for both scalar and array paths. - Registered in the datetime function `mod.rs` (UDF macro, doc export, `functions()` list). ## Are these changes tested? Yes: - Rust unit tests: return-field nullability, scalar evaluation (incl. null), and array evaluation. - sqllogictest coverage in `datafusion/sqllogictest/test_files/spark/datetime/weekday.slt`: scalar dates for all 7 weekdays, array input, TIMESTAMP / TIMESTAMP_NTZ / LTZ coercion, null handling, and argument-type / zero-argument error cases. The original PySpark 3.5.5 reference (`weekday('2009-07-30') = 3`) is preserved and validated. ## Are there any user-facing changes? Adds the new Spark-compatible scalar function `weekday`. No breaking changes. --- Note: I noticed there is an existing draft PR #22601 for the same function (last updated 2026-05-28). It appears to be a stale draft, so I've opened this as a complete, tested implementation — happy to defer or collaborate if the draft author is still active. Signed-off-by: sjhddh <151469562+sjhddh@users.noreply.github.com> Co-authored-by: sjhddh <151469562+sjhddh@users.noreply.github.com> Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
## Which issue does this PR close? - Closes #22779 ## Rationale for this change A partial aggregate with no group by expression emits one row per output partition, even for input partitions that did not receive any rows. The stats code gets this incorrect, and claims that the partial agg outputs `Exact(0)` rows in this scenario. This is off by a factor of `partition_count`, which can lead to suboptimal planning decisions downstream. Because `total_byte_size` is scaled from the output row count, this is also improved: the byte-size estimate now scales with the corrected output row count rather than the input row count. ## What changes are included in this PR? * Pass the requested partition into `statistics_inner` and adjust `statistics_inner` as described above * Tighten optimizer metadata for grouping sets queries (minor correctness fix) * Add/extend existing unit tests ## Are these changes tested? Yes; new tests added. ## Are there any user-facing changes? No.
…ions (#22784) ## Which issue does this PR close? Closes #22781 ## Rationale for this change `map(['a','b'], [col, col * 10])` fails at execution time with "map requires key and value lists to have the same length" when keys are all literals and values contain column references. Root cause: literal keys evaluate to `ColumnarValue::Scalar(FixedSizeList[N])` (length N) while column values evaluate to `ColumnarValue::Array` (length batch_size). The length check compares N != batch_size and errors. ## What changes are included in this PR? In `make_map_batch`: when one argument is scalar and the other is array, expand the scalar to match `batch_size` via `ScalarValue::to_array_of_size(number_rows)` before the length comparison. ## Are these changes tested? Yes. - Unit test: scalar keys + array values in `map.rs` - SLT test: `SELECT map(['a','b'], [column1, column1 * 10]) FROM (VALUES (1), (2), (3))` ## Are there any user-facing changes? No. Previously-failing queries now execute correctly. --------- Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
) ## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123. --> - Closes #. ## Rationale for this change <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> `array_remove` errored out when given a `NULL` array argument. `array_replace` handled the `DataType::Null` case, but it always built a NULL array of length 1, regardless of the actual input length. `remove` and `replace` share nearly identical implementations, so they should treat a NULL array argument the same way ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> - `array_remove`: handle `DataType::Null` in both `array_remove_internal` and the scalar fast-path `array_remove_with_scalar_args`, returning a NULL array of the input length. - `array_replace`: fix the existing `DataType::Null` branches to use the input array length (`array.len()` / `list_array.len()`) instead of a hard-coded `1`. ## Are these changes tested? Yes, added slt coverage <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? `array_remove`/`array_remove_n`/`array_remove_all` now return `NULL` for a `NULL`-typed array argument instead of raising an error. No breaking API changes. <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. -->
## Which issue does this PR close? Part of #21706 ## Rationale for this change Continue work on sql benchmark migration. ## What changes are included in this PR? sort_tpch sql benchmark ## Are these changes tested? Yes `BENCH_NAME=sort_tpch cargo bench --bench sql` `BENCH_NAME=sort_tpch BENCH_SIZE=10 cargo bench --bench sql` `BENCH_NAME=sort_tpch LIMIT=true cargo bench --bench sql` `BENCH_NAME=sort_tpch BENCH_SIZE=10 LIMIT=true cargo bench --bench sql` ## Are there any user-facing changes? No --------- Co-authored-by: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com>
## Which issue does this PR close? - Closes #22567 ## Rationale for this change `nanvl`'s function signature listed `Float16` first, which meant that data types not natively supported by the function (e.g., integers) would be converted to `Float16`. This resulted in unnecessary and incorrect rounding for large integer inputs (e.g., `nanvl(16777217, 1)` returned incorrect results). Instead, we now convert non-floating point inputs to `Float64`. This might still round for inputs larger than 2^53, but it's the best we can do without more fundamental changes. Floating point inputs are coerced to the widest float type they have in common -- e.g., (`Float16`, `Float32`) -> `Float32`. ## What changes are included in this PR? * Change function signature to avoid unnecessary rounding * Update SLT ## Are these changes tested? Yes; new tests added. ## Are there any user-facing changes? Yes; `nanvl` now avoids rounding in more cases. --------- Co-authored-by: Kumar Ujjawal <ujjawalpathak6@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )