You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "rank-p (via GitHub)" <gi...@apache.org> on 2026/10/13 18:22:46 UTC
[PR] fix: include nulls in file ordering bounds [datafusion]
rank-p opened a new pull request, #26280:
URL: https://github.com/apache/datafusion/pull/26280
## Which issue does this PR close?
Closes #26278.
## Rationale for this change
Ignoring NULLs in file bounds can make a combined scan appear sorted, causing `ORDER BY` to return rows in the wrong order.
## What changes are included in this PR?
Include NULLs in file bounds using NULL counts, schema nullability, and sort options. Keep the existing sort-pushdown policy.
## What is the testing strategy for this PR?
- Add a regression covering both sort directions, NULL placement, and exact/missing/estimated NULL counts. It fails without the fix.
- Passed 184 crate tests, 12,580 extended Rust tests, 533 SQL test files, and 110 CLI tests.
- CI-style lint checks pass except the security audit, which flags five existing `quinn-proto 0.11.15` advisories. Separate `--all-features` Clippy warnings remain in unchanged optimizer code.
- Local file-grouping benchmark: approximately 1.60 → 1.67 ms for 1,000 files.
## Are there any user-facing changes?
Correct NULL placement when sorting across files. No public API changes.
AI assistance: Codex.
--
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: include nulls in file ordering bounds [datafusion]
Posted by "rank-p (via GitHub)" <gi...@apache.org>.
rank-p commented on PR #26280:
URL: https://github.com/apache/datafusion/pull/26280#issuecomment-6175770311
The contributor guide says to tag a committer for CI runs/review. @xudong96 can you help with this maybe?
--
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: include nulls in file ordering bounds [datafusion]
Posted by "rank-p (via GitHub)" <gi...@apache.org>.
rank-p commented on PR #26280:
URL: https://github.com/apache/datafusion/pull/26280#issuecomment-6175685833
> The NULL-aware bounds and four-way regression test look correct. I verified that reverting only the production change makes the new test fail.
>
> One non-blocking coverage suggestion: could we add an all-NULL file next to a non-NULL file? That would exercise the new `all_null` branch and distinguish a NULL-only point interval from an exact-empty file.
Added
--
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: include nulls in file ordering bounds [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 merged PR #26280:
URL: https://github.com/apache/datafusion/pull/26280
--
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: include nulls in file ordering bounds [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #26280:
URL: https://github.com/apache/datafusion/pull/26280#issuecomment-6258103456
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/26280?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `97.32143%` with `3 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 82.80%. Comparing base ([`0d79252`](https://app.codecov.io/gh/apache/datafusion/commit/0d79252f61bd8777990b2749c216ffd07079c30d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`61c57a6`](https://app.codecov.io/gh/apache/datafusion/commit/61c57a68b0c6de226300391564044d1362b8f5f9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 62 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/26280?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [datafusion/datasource/src/file\_scan\_config/mod.rs](https://app.codecov.io/gh/apache/datafusion/pull/26280?src=pr&el=tree&filepath=datafusion%2Fdatasource%2Fsrc%2Ffile_scan_config%2Fmod.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9kYXRhc291cmNlL3NyYy9maWxlX3NjYW5fY29uZmlnL21vZC5ycw==) | 97.43% | [0 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/26280?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [datafusion/datasource/src/statistics.rs](https://app.codecov.io/gh/apache/datafusion/pull/26280?src=pr&el=tree&filepath=datafusion%2Fdatasource%2Fsrc%2Fstatistics.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9kYXRhc291cmNlL3NyYy9zdGF0aXN0aWNzLnJz) | 97.05% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/26280?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #26280 +/- ##
========================================
Coverage 82.80% 82.80%
========================================
Files 1149 1149
Lines 454146 454248 +102
Branches 454146 454248 +102
========================================
+ Hits 376035 376129 +94
- Misses 55260 55262 +2
- Partials 22851 22857 +6
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/26280?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: include nulls in file ordering bounds [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26280:
URL: https://github.com/apache/datafusion/pull/26280#issuecomment-6301226525
<!-- semver-check-comment -->
Thank you for opening this pull request!
Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks) reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).
<details>
<summary>Details</summary>
```
Cloning apache/main
Building datafusion-datasource v55.1.0 (current)
Built [ 48.397s] (current)
Parsing datafusion-datasource v55.1.0 (current)
Parsed [ 0.035s] (current)
Building datafusion-datasource v55.1.0 (baseline)
Built [ 47.889s] (baseline)
Parsing datafusion-datasource v55.1.0 (baseline)
Parsed [ 0.034s] (baseline)
Checking datafusion-datasource v55.1.0 -> v55.1.0 (no change; assume patch)
Checked [ 0.288s] 229 checks: 228 pass, 1 fail, 0 warn, 31 skip
--- failure inherent_method_missing: pub method removed or renamed ---
Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/inherent_method_missing.ron
Failed in:
FileScanConfig::preserves_file_order, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/aed9fa7af498a4f7de16022fefffc307fefd25b3/datafusion/datasource/src/file_scan_config/mod.rs:1264
Summary semver requires new major version: 1 major and 0 minor checks failed
Finished [ 98.517s] datafusion-datasource
```
</details>
--
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