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

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

@@ -52,7 +52,7 @@ homepage = "https://datafusion.apache.org"
 license = "Apache-2.0"
 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 Physical Expressions 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 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
27 checks passed

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

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

edited

Loading

jayzhan211 commented Jun 5, 2024 •

edited

Loading

alamb commented Jun 11, 2024

jayzhan211 commented Jun 12, 2024

alamb left a comment

alamb Jun 12, 2024

jayzhan211 commented Jun 12, 2024

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

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

Conversation

jayzhan211 commented May 13, 2024 • edited Loading

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?

jayzhan211 commented Jun 5, 2024 • edited Loading

alamb commented Jun 11, 2024

jayzhan211 commented Jun 12, 2024

alamb left a comment

Choose a reason for hiding this comment

alamb Jun 12, 2024

Choose a reason for hiding this comment

jayzhan211 commented Jun 12, 2024

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

edited

Loading

jayzhan211 commented Jun 5, 2024 •

edited

Loading