Move `Count` to `functions-aggregate`, update MSRV to rust 1.75 by jayzhan211 · Pull Request #10484 · apache/datafusion

jayzhan211 · 2024-05-13T12:54:58Z

Which issue does this PR close?

Part of #8708

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

There are 3 todos after this PR,

remove builtin aggregate function Count:
remove expr_fn::count and count distinct
rename count's name lowercase

I will file an issue before merging this one

Signed-off-by: jayzhan211 <[email protected]>

jayzhan211 · 2024-06-05T09:45:31Z

error: package `clap_builder v4.5.2` cannot be built because it requires rustc 1.74 or newer, while the currently active rustc version is 1.73.0
Either upgrade to rustc 1.74 or newer, or use
cargo update -p [email protected] --precise ver
where `ver` is the latest version of `clap_builder` supporting rustc 1.73.0

clap_builder needs 1.74, which used in criterion v0.5.1 (in datafusion-physical-expr)

│   │   ├── petgraph v0.6.5
│   │   │   ├── fixedbitset v0.4.2
│   │   │   └── indexmap v2.2.6 (*)
│   │   └── regex v1.10.4 (*)
│   │   [dev-dependencies]
│   │   ├── arrow v51.0.0 (*)
│   │   ├── criterion v0.5.1
│   │   │   ├── anes v0.1.6
│   │   │   ├── cast v0.3.0
│   │   │   ├── ciborium v0.2.2
│   │   │   │   ├── ciborium-io v0.2.2
│   │   │   │   ├── ciborium-ll v0.2.2
│   │   │   │   │   ├── ciborium-io v0.2.2
│   │   │   │   │   └── half v2.4.1 (*)
│   │   │   │   └── serde v1.0.203 (*)
│   │   │   ├── clap v4.5.4
│   │   │   │   ├── clap_builder v4.5.2
│   │   │   │   │   ├── anstream v0.6.14 (*)
│   │   │   │   │   ├── anstyle v1.0.7
│   │   │   │   │   ├── clap_lex v0.7.0
│   │   │   │   │   └── strsim v0.11.1
│   │   │   │   └── clap_derive v4.5.4 (proc-macro)
│   │   │   │       ├── heck v0.5.0
│   │   │   │       ├── proc-macro2 v1.0.85 (*)
│   │   │   │       ├── quote v1.0.36 (*)
│   │   │   │       └── syn v2.0.66 (*)

I upgrade to 1.75 to the latest available version.

I'm not sure why we have this issue in this PR 🤔

Signed-off-by: jayzhan211 <[email protected]>

alamb · 2024-06-11T15:29:09Z

This PR has several conflicts -- other than those, do you think this PR is ready for another review @jayzhan211 ?

Signed-off-by: jayzhan211 <[email protected]>

jayzhan211 · 2024-06-12T00:47:00Z

This PR has several conflicts -- other than those, do you think this PR is ready for another review @jayzhan211 ?

Yes, it is ready for review.

Signed-off-by: jayzhan211 <[email protected]>

alamb

Awesome -- thank you @jayzhan211 -- let's do it.

alamb · 2024-06-12T22:27:35Z

Cargo.toml

 readme = "README.md"
 repository = "https://github.com/apache/datafusion"
-rust-version = "1.73"
+rust-version = "1.75"


By my reading of https://github.com/apache/datafusion?tab=readme-ov-file#rust-version-compatibility-policy

is that since 1.74 was released in Nov 2023, we can safely update in this PR

jayzhan211 · 2024-06-12T23:54:47Z

Thanks @alamb !

…he#10484) * mv accumulate indices Signed-off-by: jayzhan211 <[email protected]> * complete udaf Signed-off-by: jayzhan211 <[email protected]> * register Signed-off-by: jayzhan211 <[email protected]> * fix expr Signed-off-by: jayzhan211 <[email protected]> * filter distinct count Signed-off-by: jayzhan211 <[email protected]> * todo: need to move count distinct too Signed-off-by: jayzhan211 <[email protected]> * move code around Signed-off-by: jayzhan211 <[email protected]> * move distinct to aggr-crate Signed-off-by: jayzhan211 <[email protected]> * replace Signed-off-by: jayzhan211 <[email protected]> * backup Signed-off-by: jayzhan211 <[email protected]> * fix function name and physical expr Signed-off-by: jayzhan211 <[email protected]> * fix physical optimizer Signed-off-by: jayzhan211 <[email protected]> * fix all slt Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * fix with args Signed-off-by: jayzhan211 <[email protected]> * add label Signed-off-by: jayzhan211 <[email protected]> * revert builtin related code back Signed-off-by: jayzhan211 <[email protected]> * fix test Signed-off-by: jayzhan211 <[email protected]> * fix substrait Signed-off-by: jayzhan211 <[email protected]> * fix doc Signed-off-by: jayzhan211 <[email protected]> * fmy Signed-off-by: jayzhan211 <[email protected]> * fix Signed-off-by: jayzhan211 <[email protected]> * fix udaf macro for distinct but not apply Signed-off-by: jayzhan211 <[email protected]> * fmt Signed-off-by: jayzhan211 <[email protected]> * fix count distinct and use workspace Signed-off-by: jayzhan211 <[email protected]> * add reverse Signed-off-by: jayzhan211 <[email protected]> * remove old code Signed-off-by: jayzhan211 <[email protected]> * backup Signed-off-by: jayzhan211 <[email protected]> * use macro Signed-off-by: jayzhan211 <[email protected]> * expr builder Signed-off-by: jayzhan211 <[email protected]> * introduce expr builder Signed-off-by: jayzhan211 <[email protected]> * add example Signed-off-by: jayzhan211 <[email protected]> * fmt Signed-off-by: jayzhan211 <[email protected]> * clean agg sta Signed-off-by: jayzhan211 <[email protected]> * combine agg Signed-off-by: jayzhan211 <[email protected]> * limit distinct and fmt Signed-off-by: jayzhan211 <[email protected]> * cleanup name Signed-off-by: jayzhan211 <[email protected]> * fix ci Signed-off-by: jayzhan211 <[email protected]> * fix window Signed-off-by: jayzhan211 <[email protected]> * fmt Signed-off-by: jayzhan211 <[email protected]> * fix ci Signed-off-by: jayzhan211 <[email protected]> * fmt Signed-off-by: jayzhan211 <[email protected]> * fix merged Signed-off-by: jayzhan211 <[email protected]> * fix Signed-off-by: jayzhan211 <[email protected]> * fix rebase Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * use std Signed-off-by: jayzhan211 <[email protected]> * update mrsv Signed-off-by: jayzhan211 <[email protected]> * upd msrv Signed-off-by: jayzhan211 <[email protected]> * revert test Signed-off-by: jayzhan211 <[email protected]> * fmt Signed-off-by: jayzhan211 <[email protected]> * downgrade to 1.75 Signed-off-by: jayzhan211 <[email protected]> * 1.76 Signed-off-by: jayzhan211 <[email protected]> * ahas Signed-off-by: jayzhan211 <[email protected]> * revert to 1.75 Signed-off-by: jayzhan211 <[email protected]> * rm count Signed-off-by: jayzhan211 <[email protected]> * fix merge Signed-off-by: jayzhan211 <[email protected]> * fmt Signed-off-by: jayzhan211 <[email protected]> * clippy Signed-off-by: jayzhan211 <[email protected]> * rm sum in test_no_duplicate_name Signed-off-by: jayzhan211 <[email protected]> * fix Signed-off-by: jayzhan211 <[email protected]> --------- Signed-off-by: jayzhan211 <[email protected]>

jayzhan211 added 15 commits May 11, 2024 10:59

mv accumulate indices

755215d

Signed-off-by: jayzhan211 <[email protected]>

complete udaf

12366e6

Signed-off-by: jayzhan211 <[email protected]>

register

9f43638

Signed-off-by: jayzhan211 <[email protected]>

fix expr

7dd2aae

Signed-off-by: jayzhan211 <[email protected]>

filter distinct count

2fb0c2a

Signed-off-by: jayzhan211 <[email protected]>

todo: need to move count distinct too

9dd044c

Signed-off-by: jayzhan211 <[email protected]>

move code around

ab9eaaa

Signed-off-by: jayzhan211 <[email protected]>

move distinct to aggr-crate

1506453

Signed-off-by: jayzhan211 <[email protected]>

replace

6a554ee

Signed-off-by: jayzhan211 <[email protected]>

Merge branch 'count-udaf' into count-for-all

dae3061

backup

a44c967

Signed-off-by: jayzhan211 <[email protected]>

fix function name and physical expr

dd5a90f

Signed-off-by: jayzhan211 <[email protected]>

fix physical optimizer

c9eebb3

Signed-off-by: jayzhan211 <[email protected]>

fix all slt

6375d47

Signed-off-by: jayzhan211 <[email protected]>

cleanup

37d8974

Signed-off-by: jayzhan211 <[email protected]>

github-actions bot added sql SQL Planner logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates optimizer Optimizer rules core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels May 13, 2024

jayzhan211 added 7 commits May 13, 2024 20:58

Merge remote-tracking branch 'upstream/main' into count-for-all

643ffe9

cleanup

9917b37

Signed-off-by: jayzhan211 <[email protected]>

fix with args

c6ba32b

Signed-off-by: jayzhan211 <[email protected]>

add label

2d80a8b

Signed-off-by: jayzhan211 <[email protected]>

revert builtin related code back

2bcb10b

Signed-off-by: jayzhan211 <[email protected]>

fix test

12f27c4

Signed-off-by: jayzhan211 <[email protected]>

fix substrait

a391407

Signed-off-by: jayzhan211 <[email protected]>

github-actions bot added the substrait Changes to the substrait crate label May 14, 2024

fix doc

3ace4bf

Signed-off-by: jayzhan211 <[email protected]>

rm count

ea864e0

Signed-off-by: jayzhan211 <[email protected]>

jayzhan211 marked this pull request as ready for review June 5, 2024 06:34

jayzhan211 requested a review from alamb June 5, 2024 06:35

jayzhan211 added 6 commits June 7, 2024 07:35

Merge remote-tracking branch 'upstream/main' into count-for-all

d621fb6

Merge remote-tracking branch 'upstream/main' into count-for-all

b3f8f8c

Merge remote-tracking branch 'upstream/main' into count-for-all

121e4cb

fix merge

d89424d

Signed-off-by: jayzhan211 <[email protected]>

Merge remote-tracking branch 'upstream/main' into count-for-all

a88916d

fmt

0924248

Signed-off-by: jayzhan211 <[email protected]>

jayzhan211 added 2 commits June 12, 2024 08:42

Merge remote-tracking branch 'upstream/main' into count-for-all

33a02f9

clippy

4b13fc0

Signed-off-by: jayzhan211 <[email protected]>

rm sum in test_no_duplicate_name

4bd680d

Signed-off-by: jayzhan211 <[email protected]>

jayzhan211 marked this pull request as draft June 12, 2024 07:53

jayzhan211 force-pushed the count-for-all branch 2 times, most recently from 45249fb to 4bd680d Compare June 12, 2024 13:48

jayzhan211 added 2 commits June 12, 2024 21:49

Merge remote-tracking branch 'upstream/main' into count-for-all

cf6f9ac

fix

97a1227

Signed-off-by: jayzhan211 <[email protected]>

jayzhan211 marked this pull request as ready for review June 12, 2024 14:21

alamb changed the title ~~Move Count to functions-aggregate~~ Move Count to functions-aggregate, update MSRV Jun 12, 2024

alamb changed the title ~~Move Count to functions-aggregate, update MSRV~~ Move Count to functions-aggregate, update MSRV to rust 1.75 Jun 12, 2024

alamb approved these changes Jun 12, 2024

View reviewed changes

jayzhan211 merged commit 8f718dd into apache:main Jun 12, 2024

jayzhan211 deleted the count-for-all branch June 12, 2024 23:54

findepi mentioned this pull request Jul 2, 2024

Add example of dataframe API aggregations #11219

Closed

huaxingao mentioned this pull request Jul 8, 2024

feat: Upgrade to DataFusion 40.0.0-rc1 apache/datafusion-comet#644

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Move `Count` to `functions-aggregate`, update MSRV to rust 1.75#10484

Move `Count` to `functions-aggregate`, update MSRV to rust 1.75#10484
jayzhan211 merged 81 commits intoapache:mainfrom
jayzhan211:count-for-all

jayzhan211 commented May 13, 2024 •

edited

Loading

Uh oh!

jayzhan211 commented Jun 5, 2024 •

edited

Loading

Uh oh!

alamb commented Jun 11, 2024

Uh oh!

jayzhan211 commented Jun 12, 2024

Uh oh!

alamb left a comment

Uh oh!

alamb Jun 12, 2024

Uh oh!

jayzhan211 commented Jun 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

jayzhan211 commented May 13, 2024 • 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!

jayzhan211 commented Jun 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alamb commented Jun 11, 2024

Uh oh!

jayzhan211 commented Jun 12, 2024

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb Jun 12, 2024

Choose a reason for hiding this comment

Uh oh!

jayzhan211 commented Jun 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jayzhan211 commented May 13, 2024 •

edited

Loading

jayzhan211 commented Jun 5, 2024 •

edited

Loading