Skip to content

minor: combine set_comparison test into core_integration binary - #26419

Merged
alamb merged 2 commits into
apache:mainfrom
alamb:fold-set-comparison-into-core-integration
Oct 21, 2026
Merged

alamb merged 2 commits into
apache:mainfrom
alamb:fold-set-comparison-into-core-integration

Conversation

@alamb

@alamb alamb commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

This follows the same rationale as earlier pRs like

Rationale for this change

Each .rs file directly in datafusion/core/tests becomes its own test binary. In this case, the standalone set_comparison binary requires 389 MB on my machine (macOS, aarch64) to run just 6 small tests

 cargo test -p datafusion --test set_comparison --no-run

The binary was added in

What changes are included in this PR?

Move datafusion/core/tests/set_comparison.rs to datafusion/core/tests/sql/set_comparison.rs and register it in sql/mod.rs, so it is built as part of core_integration.

Size of core_integration (debug build):

bytes
before 457,079,488
after 457,223,744 (+144 KB)

So this saves creating a ~389 MB binary at a cost of ~144 KB in core_integration.

What is the testing strategy for this PR?

CI

Are there any user-facing changes?

No, developer productivity / CI efficiency

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the core Core DataFusion crate label Oct 16, 2026
@alamb alamb added the development-process Related to development process of DataFusion label Oct 16, 2026
@alamb alamb changed the title test: fold set_comparison test into core_integration binary minor: combine set_comparison test into core_integration binary Oct 16, 2026
@alamb alamb changed the title minor: combine set_comparison test into core_integration binary minor: combine set_comparison test into core_integration binary Oct 16, 2026
@alamb
alamb marked this pull request as ready for review October 16, 2026 16:43
@codecov-commenter

codecov-commenter commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.94%. Comparing base (79650ec) to head (0535447).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #26419      +/-   ##
==========================================
- Coverage   82.94%   82.94%   -0.01%     
==========================================
  Files        1156     1156              
  Lines      458581   458581              
  Branches   458581   458581              
==========================================
- Hits       380382   380373       -9     
- Misses      55145    55153       +8     
- Partials    23054    23055       +1     

☔ 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.

bvolpato pushed a commit to bvolpato/datafusion that referenced this pull request Oct 16, 2026
…26422)

## Which issue does this PR close?

This follows the same rationale as earlier PRs like
- apache#10588
- apache#11831
- apache#18226
- apache#26419

## Rationale for this change

Each `.rs` file directly in `datafusion/core/tests` becomes its own test
binary. In this case, the standalone `tpcds_planning` binary requires
**390 MB** (408,695,296 bytes, as reported by `du -h`) on my machine
(macOS, aarch64)

```shell
cargo test -p datafusion --test tpcds_planning --no-run
```

The binary was added in
- apache#4719

## What changes are included in this PR?

Move `datafusion/core/tests/tpcds_planning.rs` to
`datafusion/core/tests/sql/tpcds_planning.rs` and register it in
`sql/mod.rs`, so it is built as part of `core_integration`.

Size of `core_integration` (debug build):

| | bytes |
|---|---|
| before | 457,595,344 |
| after | 458,314,960 (+703 KB) |

So this saves creating a ~390 MB binary at a cost of ~703 KB in
`core_integration`.

## What is the testing strategy for this PR?
CI

## Are there any user-facing changes?

No, developer productivity / CI efficiency

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot removed the development-process Related to development process of DataFusion label Oct 20, 2026

@nuno-faria nuno-faria 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.

Also LGTM, thanks. CI is a little slow today, but it should pass.

zzcclp pushed a commit to zzcclp/arrow-datafusion that referenced this pull request Oct 21, 2026
…che#26423)

## Which issue does this PR close?

This follows the same rationale as earlier PRs like
- apache#10588
- apache#11831
- apache#18226
- apache#26419

## Rationale for this change

Each `.rs` file directly in `datafusion/ffi/tests` becomes its own test
binary. In this case, the 10 standalone `ffi_*` binaries require about
**2.48 GB** (2,664,138,160 bytes) in total on my machine (macOS,
aarch64)

```shell
cargo test -p datafusion-ffi --features integration-tests --tests --no-run
```

The binaries were added in
- `ffi_integration`: apache#14579
- `ffi_udaf`: apache#14775
- `ffi_udf`: apache#15487
- `ffi_udtf`: apache#15581
- `ffi_udwf`: apache#16261
- `ffi_catalog`: apache#18657
- `ffi_config`: apache#19469
- `ffi_execution_plan`: apache#20449
- `ffi_physical_optimizer`: apache#20451
- `ffi_query_planner`: apache#24028

## What changes are included in this PR?

Combine the 10 `datafusion/ffi/tests/ffi_*.rs` files into the single
`ffi_integration` test binary:

Size of the binaries (debug build):

| binary | bytes |
|---|---|
| `ffi_catalog` | 382,295,824 |
| `ffi_config` | 6,295,496 |
| `ffi_execution_plan` | 71,841,312 |
| `ffi_integration` (before) | 382,373,904 |
| `ffi_physical_optimizer` | 71,361,096 |
| `ffi_query_planner` | 382,445,168 |
| `ffi_udaf` | 320,780,232 |
| `ffi_udf` | 319,111,528 |
| `ffi_udtf` | 407,588,120 |
| `ffi_udwf` | 320,045,480 |
| **total before (10 binaries)** | **2,664,138,160** |
| **`ffi_integration` after (1 binary)** | **408,453,600** |

So this goes from about 2.48 GB to about 390 MB (408,453,600 bytes)
(~85% smaller) and from 10 link steps to 1.

## What is the testing strategy for this PR?
CI. The same 32 tests run before and after:

```shell
cargo test -p datafusion-ffi --features integration-tests --test ffi_integration
```

## Are there any user-facing changes?

No, developer productivity / CI efficiency

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
@alamb
alamb added this pull request to the merge queue Oct 21, 2026
@alamb

alamb commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Thank you @nuno-faria and @kosiew

Merged via the queue into apache:main with commit cb50961 Oct 21, 2026
43 checks passed
@alamb
alamb deleted the fold-set-comparison-into-core-integration branch October 21, 2026 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate v56.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants