Improve feature flag CI coverage `datafusion` and `datafusion-functions` by alamb · Pull Request #15203 · apache/datafusion

alamb · 2025-03-13T12:40:43Z

Which issue does this PR close?

Closes Improved CI test coverage for rust features #15155
Follow on to Split out datafusion-substrait and datafusion-proto CI feature checks, increase coverage #15156

Rationale for this change

I reviewed the CI coverage for datafusion's (many!) feature flag and it is haphazard -- for example it was not clear where to add a test for #15124

What changes are included in this PR?

I reviewed and ensured there is full feature flag coverage for the datafusion and datafusion-functions crate

Are these changes tested?

Are there any user-facing changes?

alamb · 2025-03-13T12:41:16Z

.github/workflows/rust.yml

        uses: ./.github/actions/setup-builder
        with:
          rust-version: stable
+      - name: Check datafusion-common (default features)


there was no check for default set of features (even the tests run with avro, etc) so I added that

good finding!

alamb · 2025-03-13T15:56:17Z

.github/workflows/rust.yml

      - name: Check datafusion-substrait (no-default-features)
        run: cargo check --profile ci --all-targets --no-default-features -p datafusion-substrait
+        #
+        # Note: Only check libraries (not --all-targets) to cover end user APIs


The idea is to test only --all-targets for the default features (what a developer would likely run), otherwise the CI will just verify that a user of the crate can compile things rather than check all tests and benches too

alamb · 2025-03-13T20:13:24Z

@xudong963 I wonder if you have some time to review this PR?

xudong963

Sorry for late, LGTM

alamb · 2025-03-17T14:38:51Z

Thank you for the review @xudong963 -- much apprecaited

…ns` (apache#15203)

* [draft] add shot circuit in BinaryExpr * refactor: add check_short_circuit function * refactor: change if condition to match * feat: Add support for --mem-pool-type and --memory-limit options to multiple benchmarks (#14642) * Add support --mem-pool-type and --memory-limit options for all benchmarks * Add --sort-spill-reservation-bytes option * Chore/Add additional FFI unit tests (#14802) * Add unit tests to FFI_ExecutionPlan * Add unit tests for FFI table source * Add round trip tests for volatility * Add unit tests for FFI insert op * Simplify string generation in unit test Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Fix drop of borrowed value --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Improve feature flag CI coverage `datafusion` and `datafusion-functions` (#15203) * add extend sql & docs * feat: Add support for --mem-pool-type and --memory-limit options to multiple benchmarks (#14642) * Add support --mem-pool-type and --memory-limit options for all benchmarks * Add --sort-spill-reservation-bytes option * Chore/Add additional FFI unit tests (#14802) * Add unit tests to FFI_ExecutionPlan * Add unit tests for FFI table source * Add round trip tests for volatility * Add unit tests for FFI insert op * Simplify string generation in unit test Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Fix drop of borrowed value --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Improve feature flag CI coverage `datafusion` and `datafusion-functions` (#15203) * fix: incorrect false judgment * add test * separate q6 to new PR * feat: Add support for --mem-pool-type and --memory-limit options to multiple benchmarks (#14642) * Add support --mem-pool-type and --memory-limit options for all benchmarks * Add --sort-spill-reservation-bytes option * Chore/Add additional FFI unit tests (#14802) * Add unit tests to FFI_ExecutionPlan * Add unit tests for FFI table source * Add round trip tests for volatility * Add unit tests for FFI insert op * Simplify string generation in unit test Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Fix drop of borrowed value --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Improve feature flag CI coverage `datafusion` and `datafusion-functions` (#15203) * feat: Add support for --mem-pool-type and --memory-limit options to multiple benchmarks (#14642) * Add support --mem-pool-type and --memory-limit options for all benchmarks * Add --sort-spill-reservation-bytes option * Chore/Add additional FFI unit tests (#14802) * Add unit tests to FFI_ExecutionPlan * Add unit tests for FFI table source * Add round trip tests for volatility * Add unit tests for FFI insert op * Simplify string generation in unit test Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Fix drop of borrowed value --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Improve feature flag CI coverage `datafusion` and `datafusion-functions` (#15203) * add benchmark for boolean_op * fix cargo doc * add binary_op bench * Better comments --------- Co-authored-by: Kristin Cowalcijk <bo@wherobots.com> Co-authored-by: Tim Saucer <timsaucer@gmail.com> Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

* [draft] add shot circuit in BinaryExpr * refactor: add check_short_circuit function * refactor: change if condition to match * feat: Add support for --mem-pool-type and --memory-limit options to multiple benchmarks (apache#14642) * Add support --mem-pool-type and --memory-limit options for all benchmarks * Add --sort-spill-reservation-bytes option * Chore/Add additional FFI unit tests (apache#14802) * Add unit tests to FFI_ExecutionPlan * Add unit tests for FFI table source * Add round trip tests for volatility * Add unit tests for FFI insert op * Simplify string generation in unit test Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Fix drop of borrowed value --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Improve feature flag CI coverage `datafusion` and `datafusion-functions` (apache#15203) * add extend sql & docs * feat: Add support for --mem-pool-type and --memory-limit options to multiple benchmarks (apache#14642) * Add support --mem-pool-type and --memory-limit options for all benchmarks * Add --sort-spill-reservation-bytes option * Chore/Add additional FFI unit tests (apache#14802) * Add unit tests to FFI_ExecutionPlan * Add unit tests for FFI table source * Add round trip tests for volatility * Add unit tests for FFI insert op * Simplify string generation in unit test Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Fix drop of borrowed value --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Improve feature flag CI coverage `datafusion` and `datafusion-functions` (apache#15203) * fix: incorrect false judgment * add test * separate q6 to new PR * feat: Add support for --mem-pool-type and --memory-limit options to multiple benchmarks (apache#14642) * Add support --mem-pool-type and --memory-limit options for all benchmarks * Add --sort-spill-reservation-bytes option * Chore/Add additional FFI unit tests (apache#14802) * Add unit tests to FFI_ExecutionPlan * Add unit tests for FFI table source * Add round trip tests for volatility * Add unit tests for FFI insert op * Simplify string generation in unit test Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Fix drop of borrowed value --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Improve feature flag CI coverage `datafusion` and `datafusion-functions` (apache#15203) * feat: Add support for --mem-pool-type and --memory-limit options to multiple benchmarks (apache#14642) * Add support --mem-pool-type and --memory-limit options for all benchmarks * Add --sort-spill-reservation-bytes option * Chore/Add additional FFI unit tests (apache#14802) * Add unit tests to FFI_ExecutionPlan * Add unit tests for FFI table source * Add round trip tests for volatility * Add unit tests for FFI insert op * Simplify string generation in unit test Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Fix drop of borrowed value --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Improve feature flag CI coverage `datafusion` and `datafusion-functions` (apache#15203) * add benchmark for boolean_op * fix cargo doc * add binary_op bench * Better comments --------- Co-authored-by: Kristin Cowalcijk <bo@wherobots.com> Co-authored-by: Tim Saucer <timsaucer@gmail.com> Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

github-actions bot added the development-process Related to development process of DataFusion label Mar 13, 2025

alamb commented Mar 13, 2025

View reviewed changes

alamb changed the title ~~Improve feature flag CI coverage datafusion and datafusion-functions~~ Improve feature flag CI coverage datafusion and datafusion-functions, remove core_expressions feature Mar 13, 2025

github-actions bot added the functions Changes to functions implementation label Mar 13, 2025

alamb force-pushed the alamb/features2 branch from a02b1be to 18de2aa Compare March 13, 2025 15:12

github-actions bot removed the functions Changes to functions implementation label Mar 13, 2025

alamb changed the title ~~Improve feature flag CI coverage datafusion and datafusion-functions, remove core_expressions feature~~ Improve feature flag CI coverage datafusion and datafusion-functions Mar 13, 2025

github-actions bot added the core Core DataFusion crate label Mar 13, 2025

alamb commented Mar 13, 2025

View reviewed changes

Improve feature flag CI coverage datafusion and datafusion-functions

e31b779

alamb force-pushed the alamb/features2 branch from feaa790 to e31b779 Compare March 13, 2025 16:01

alamb marked this pull request as ready for review March 13, 2025 16:21

alamb mentioned this pull request Mar 13, 2025

feat: Add ConfigOptions to ScalarFunctionArgs #13527

Closed

Merge remote-tracking branch 'apache/main' into alamb/features2

6945c76

xudong963 approved these changes Mar 16, 2025

View reviewed changes

Merge remote-tracking branch 'apache/main' into alamb/features2

53e97ff

alamb added a commit to alamb/datafusion that referenced this pull request Mar 17, 2025

Improve feature flag CI coverage datafusion and `datafusion-functio…

db871af

…ns` (apache#15203)

alamb merged commit 1d0c9cb into apache:main Mar 17, 2025
4 checks passed

alamb deleted the alamb/features2 branch March 17, 2025 15:35

acking-you pushed a commit to acking-you/arrow-datafusion that referenced this pull request Mar 27, 2025

Improve feature flag CI coverage datafusion and `datafusion-functio…

4a199c1

…ns` (apache#15203)

acking-you pushed a commit to acking-you/arrow-datafusion that referenced this pull request Mar 28, 2025

Improve feature flag CI coverage datafusion and `datafusion-functio…

2599ab4

…ns` (apache#15203)

acking-you pushed a commit to acking-you/arrow-datafusion that referenced this pull request Mar 29, 2025

Improve feature flag CI coverage datafusion and `datafusion-functio…

f5e7a29

…ns` (apache#15203)

acking-you pushed a commit to acking-you/arrow-datafusion that referenced this pull request Mar 29, 2025

Improve feature flag CI coverage datafusion and `datafusion-functio…

398a5b4

…ns` (apache#15203)

acking-you pushed a commit to acking-you/arrow-datafusion that referenced this pull request Mar 31, 2025

Improve feature flag CI coverage datafusion and `datafusion-functio…

f171227

…ns` (apache#15203)

acking-you pushed a commit to acking-you/arrow-datafusion that referenced this pull request Mar 31, 2025

Improve feature flag CI coverage datafusion and `datafusion-functio…

2205edb

…ns` (apache#15203)

acking-you pushed a commit to acking-you/arrow-datafusion that referenced this pull request Apr 7, 2025

Improve feature flag CI coverage datafusion and `datafusion-functio…

0ef29b1

…ns` (apache#15203)

acking-you pushed a commit to acking-you/arrow-datafusion that referenced this pull request Apr 7, 2025

Improve feature flag CI coverage datafusion and `datafusion-functio…

13742d2

…ns` (apache#15203)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve feature flag CI coverage `datafusion` and `datafusion-functions`#15203

Improve feature flag CI coverage `datafusion` and `datafusion-functions`#15203
alamb merged 3 commits intoapache:mainfrom
alamb:alamb/features2

alamb commented Mar 13, 2025 •

edited

Loading

Uh oh!

alamb Mar 13, 2025

Uh oh!

xudong963 Mar 16, 2025

Uh oh!

alamb Mar 13, 2025 •

edited

Loading

Uh oh!

alamb commented Mar 13, 2025

Uh oh!

xudong963 left a comment

Uh oh!

alamb commented Mar 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

alamb commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

alamb Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

xudong963 Mar 16, 2025

Choose a reason for hiding this comment

Uh oh!

alamb Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb commented Mar 13, 2025

Uh oh!

xudong963 left a comment

Choose a reason for hiding this comment

Uh oh!

alamb commented Mar 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alamb commented Mar 13, 2025 •

edited

Loading

alamb Mar 13, 2025 •

edited

Loading