Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-11323: [Rust][DataFusion] Allow sort queries to return no results #9275

Closed
wants to merge 1 commit into from

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Jan 20, 2021

Prior to this PR, if a plan had an ORDER BY (Sort) that got no input rows, you would get an output error.

Now the test passes and produces the (expected) no output rows

@github-actions
Copy link

@codecov-io
Copy link

Codecov Report

Merging #9275 (0297af3) into master (6912869) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #9275   +/-   ##
=======================================
  Coverage   81.68%   81.68%           
=======================================
  Files         215      215           
  Lines       52561    52577   +16     
=======================================
+ Hits        42935    42949   +14     
- Misses       9626     9628    +2     
Impacted Files Coverage Δ
rust/datafusion/src/execution/context.rs 89.73% <100.00%> (+0.16%) ⬆️
rust/datafusion/src/physical_plan/sort.rs 89.26% <100.00%> (-0.39%) ⬇️
rust/parquet/src/encodings/encoding.rs 94.86% <0.00%> (-0.20%) ⬇️

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 6912869...0297af3. Read the comment docs.

@@ -141,7 +141,10 @@ fn sort_batches(
batches: &Vec<RecordBatch>,
schema: &SchemaRef,
expr: &[PhysicalSortExpr],
) -> ArrowResult<RecordBatch> {
) -> ArrowResult<Option<RecordBatch>> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I did not use create_empty_record_batch to create a 0 row record batch (the approach taken in hash_aggregrate.rs) https://github.com/apache/arrow/blob/master/rust/datafusion/src/physical_plan/hash_aggregate.rs#L827, as the approach in this PR will support any data type now or in the future.

@alamb
Copy link
Contributor Author

alamb commented Jan 21, 2021

@jorgecarleitao and @andygrove -- if you have time, could you review this (small PR)? It is something I hit while trying to upgrade my downstream IOx project to use the latest arrow/datafusion

Copy link
Member

@jorgecarleitao jorgecarleitao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Sorry for the delay. I had reviewed it yesterday but forgot to post here :/

@alamb
Copy link
Contributor Author

alamb commented Jan 21, 2021

Thanks @jorgecarleitao !

@alamb alamb closed this in 84126d5 Jan 21, 2021
kszucs pushed a commit that referenced this pull request Jan 25, 2021
Prior to this PR, if a plan had an ORDER BY (Sort) that got no input rows, you would get an output error.

Now the test passes and produces the (expected) no output rows

Closes #9275 from alamb/alamb/ARROW-11323-empty-results

Authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Signed-off-by: Andrew Lamb <andrew@nerdnetworks.org>
GeorgeAp pushed a commit to sirensolutions/arrow that referenced this pull request Jun 7, 2021
Prior to this PR, if a plan had an ORDER BY (Sort) that got no input rows, you would get an output error.

Now the test passes and produces the (expected) no output rows

Closes apache#9275 from alamb/alamb/ARROW-11323-empty-results

Authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Signed-off-by: Andrew Lamb <andrew@nerdnetworks.org>
michalursa pushed a commit to michalursa/arrow that referenced this pull request Jun 13, 2021
Prior to this PR, if a plan had an ORDER BY (Sort) that got no input rows, you would get an output error.

Now the test passes and produces the (expected) no output rows

Closes apache#9275 from alamb/alamb/ARROW-11323-empty-results

Authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Signed-off-by: Andrew Lamb <andrew@nerdnetworks.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants