Skip to content

feat: #110 support group by column positions - #519

Merged
alamb merged 5 commits into
apache:masterfrom
jychen7:110-group-by-position
Jun 8, 2021
Merged

feat: #110 support group by column positions#519
alamb merged 5 commits into
apache:masterfrom
jychen7:110-group-by-position

Conversation

@jychen7

@jychen7jychen7 commented Jun 6, 2021

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes#110

What changes are included in this PR?

similar to extract and resolve alias, we create new functions to extract and resolve positions
(note: alias is resolve recursively, but position is resolve as a whole)

Are there any user-facing changes?

no

@codecov-commenter

codecov-commenter commented Jun 6, 2021•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #519 (c86c061) into master (b84789a) will decrease coverage by 0.00%.
The diff coverage is 79.16%.

Impacted file tree graph

@@ Coverage Diff @@## master #519 +/- ##
==========================================
- Coverage 76.09% 76.08% -0.01% 
==========================================
Files 155 157 +2 Lines 26575 26901 +326 ==========================================
+ Hits 20221 20468 +247 - Misses 6354 6433 +79
Impacted FilesCoverage Δ
datafusion-cli/src/lib.rs0.00% <0.00%> (ø)
datafusion-cli/src/main.rs0.00% <0.00%> (ø)
datafusion/src/sql/planner.rs84.51% <90.90%> (+0.25%)⬆️
datafusion/src/sql/utils.rs67.61% <100.00%> (+0.82%)⬆️
datafusion/src/physical_plan/windows.rs86.22% <0.00%> (-0.91%)⬇️
...lista/rust/core/src/serde/logical_plan/to_proto.rs61.64% <0.00%> (-0.77%)⬇️
...sta/rust/core/src/serde/logical_plan/from_proto.rs35.41% <0.00%> (-0.51%)⬇️
datafusion/src/optimizer/utils.rs48.05% <0.00%> (-0.18%)⬇️
...ta/rust/core/src/serde/physical_plan/from_proto.rs38.51% <0.00%> (-0.14%)⬇️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee2b9ef...c86c061. Read the comment docs.

Dandandan
Dandandan reviewed Jun 6, 2021
View reviewed changes
Comment threaddatafusion/src/sql/utils.rs Outdated
Dandandan
Dandandan reviewed Jun 6, 2021
View reviewed changes
Comment threaddatafusion/src/sql/utils.rs Outdated
Dandandan
Dandandan reviewed Jun 6, 2021
View reviewed changes
Comment threaddatafusion/src/sql/utils.rs
jychen7
jychen7 commented Jun 6, 2021
View reviewed changes
Comment threaddatafusion/src/sql/utils.rs Outdated
Dandandan
Dandandan reviewed Jun 7, 2021
View reviewed changes
Comment threaddatafusion/src/sql/utils.rs Outdated
Comment threaddatafusion/src/sql/utils.rs Outdated
Comment threaddatafusion/src/sql/utils.rs Outdated
alamb
alamb reviewed Jun 7, 2021
View reviewed changes

@alambalamb 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.

This is looking good @jychen7 -- thank you for the contribution 👍 A few more tests and it will be good to go from my perspective

Comment threaddatafusion/src/sql/planner.rs
Comment threaddatafusion/src/sql/utils.rs Outdated
Comment threaddatafusion/src/sql/utils.rs
@jychen7
jychen7 requested review from Dandandan and alambJune 8, 2021 00:44
alamb
alamb reviewed Jun 8, 2021
View reviewed changes
Comment threaddatafusion/src/sql/planner.rs
\n TableScan: person projection=None",
);
quick_test(
"SELECT state, age AS b, COUNT(1) FROM person GROUP BY 2, 1",

@alambalambJun 8, 2021

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.

👍

alamb
alamb approved these changes Jun 8, 2021
View reviewed changes

@alambalamb 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.

Looks great to me @jychen7 -- thank you so much!

@alamb
alamb merged commit e39f311 into apache:masterJun 8, 2021
@houqphouqp changed the title 110 support group by positionssupport group by column positionsJul 31, 2021
@houqphouqp added enhancement New feature or request sql SQL Planner labels Jul 31, 2021
@jychen7
jychen7 deleted the 110-group-by-position branch February 13, 2022 18:11
@jychen7jychen7 changed the title support group by column positions#100 support group by column positionsMar 27, 2022
@jychen7jychen7 changed the title #100 support group by column positions#110 support group by column positionsMar 27, 2022
@jychen7jychen7 changed the title #110 support group by column positionsfeat: #110 support group by column positionsMar 27, 2022
unkloud pushed a commit to unkloud/datafusion that referenced this pull request Mar 23, 2025
@andygrove
feat: Add fuzz testing for arithmetic expressions (apache#519)
c6d387c
* Add fuzz tests for aritmetic expressions
* add unary math
* add bit-wise expressions
* bug fix
HairstonE pushed a commit to HairstonE/datafusion that referenced this pull request Oct 7, 2025
@jdye64
Allow for multiple input files per table instead of a single file (ap…
501acff
…ache#519)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alambalambalamb approved these changes
@DandandanDandandanAwaiting requested review from Dandandan

Assignees

No one assigned

Labels

enhancementNew feature or requestsqlSQL Planner

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Grouping by column position

5 participants

@jychen7@codecov-commenter@Dandandan@alamb@houqp