Skip to content

test: consolidate ASOF SQL coverage - #24964

Merged
jayzhan211 merged 15 commits into
apache:mainfrom
Xuanwo:xuanwo/asof-slt-coverage
Oct 7, 2026
Merged

jayzhan211 merged 15 commits into
apache:mainfrom
Xuanwo:xuanwo/asof-slt-coverage

Conversation

@Xuanwo

@Xuanwo Xuanwo commented Sep 6, 2026 •

Copy link
Copy Markdown
Member

Which issue does this PR close?

Rationale for this change

ASOF SQL behavior was split between sqllogictest and Rust integration tests. SQL-visible results and plan shapes are easier to review and maintain in SLT, while Rust coverage remains useful for execution setup that SLT cannot express.

What changes are included in this PR?

  • Move duplicated coercion, EXPLAIN, USING output, and validation coverage into asof_join.slt.
  • Add result and plan-shape coverage for right-output filtering, projection pruning, expression operands, empty inputs, duplicate left rows, self joins, and multi-key USING.
  • Keep Rust tests that require multi-partition physical plans, unbounded inputs, cross-batch execution, or direct plan-to-SQL roundtrips.

Are these changes tested?

Yes:

  • cargo fmt --all -- --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo test --profile ci -p datafusion-sqllogictest --test sqllogictests -- asof_join
  • cargo test --profile ci -p datafusion --test core_integration --all-features -- asof
  • Extended workspace tests from the contributor guide

Are there any user-facing changes?

No. This consolidates and expands coverage for the existing ASOF SQL contract.

@github-actions github-actions Bot added documentation Improvements or additions to documentation sql SQL Planner logical-expr Logical plan and expressions core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) substrait Changes to the substrait crate labels Sep 6, 2026
@Xuanwo Xuanwo mentioned this pull request Sep 4, 2026
@github-actions github-actions Bot added the auto detected api change Auto detected API change label Sep 6, 2026
@codecov-commenter

codecov-commenter commented Sep 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.69%. Comparing base (c0e872f) to head (702f8c0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24964      +/-   ##
==========================================
- Coverage   82.69%   82.69%   -0.01%     
==========================================
  Files        1147     1147              
  Lines      447348   447348              
  Branches   447348   447348              
==========================================
- Hits       369936   369927       -9     
- Misses      54999    55006       +7     
- Partials    22413    22415       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added development-process Related to development process of DataFusion physical-expr Changes to the physical-expr crates optimizer Optimizer rules catalog Related to the catalog crate common Related to common crate execution Related to the execution crate proto Related to proto crate labels Oct 5, 2026
@github-actions github-actions Bot added physical-plan Changes to the physical-plan crate spark labels Oct 5, 2026
@Xuanwo
Xuanwo marked this pull request as ready for review October 5, 2026 05:00
@github-actions github-actions Bot removed the auto detected api change Auto detected API change label Oct 5, 2026
@Xuanwo

Xuanwo commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

Hi @jayzhan211 and @2010YOUY01, this ASOF test coverage follow-up is now restacked and ready for review. Would you mind taking a look when you have time? Thanks!

@jayzhan211

Copy link
Copy Markdown
Contributor

This test only passes because l sorts before r. exclude_using_columns keeps the USING key whose qualifier sorts first. With x/a aliases, SELECT * emits a.grp, which is NULL for unmatched left rows and lands in column 3, while SELECT grp still returns x.grp. The deleted Rust assertion (t/p → ["ts", "trade_id", "symbol", ...]) was the only test for that case, so please keep it here:

-# SELECT * exposes the USING key once.
+# SELECT * exposes the USING key once, keeping the key whose qualifier sorts
+# first (`l` < `r` keeps the left key).
# With the right alias sorting first, SELECT * keeps the right key, which is
# NULL for unmatched left rows.
query IPTPT
SELECT *
FROM asof_left x
ASOF JOIN asof_right a
MATCH_CONDITION (x.ts >= a.ts)
USING (grp)
WHERE x.id IN (1, 2)
ORDER BY x.id;
----
1 2024-01-01T09:00:01 NULL NULL NULL
2 2024-01-01T09:00:04 A 2024-01-01T09:00:04 a4

@Xuanwo

Xuanwo commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

@jayzhan211 nice catch! Will fix.

@github-actions github-actions Bot removed documentation Improvements or additions to documentation sql SQL Planner development-process Related to development process of DataFusion logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates optimizer Optimizer rules substrait Changes to the substrait crate catalog Related to the catalog crate common Related to common crate execution Related to the execution crate proto Related to proto crate functions Changes to functions implementation datasource Changes to the datasource crate ffi Changes to the ffi crate physical-plan Changes to the physical-plan crate spark labels Oct 5, 2026

@jayzhan211 jayzhan211 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Xuanwo !

@jayzhan211
jayzhan211 added this pull request to the merge queue Oct 7, 2026
Merged via the queue into apache:main with commit 549439f Oct 7, 2026
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) v56.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants