GH-47481: [C++][Acero] record_batch_reader_source specify Ordering::Implicit to support select * limit k#47482
GH-47481: [C++][Acero] record_batch_reader_source specify Ordering::Implicit to support select * limit k#47482egolearner wants to merge 3 commits into
select * limit k#47482Conversation
|
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format? or See also: |
select * limit kselect * limit k
|
|
|
Hi @westonpace , please take a look |
1 similar comment
|
Hi @westonpace , please take a look |
|
ping @westonpace |
|
Since @westonpace seems unavailable — @jonkeane @kou @raulcd could you please review this PR? |
ca21ef4 to
cf69c13
Compare
|
ping @westonpace @jonkeane @kou @raulcd |
|
Can we add a test for this case? |
There was a problem hiding this comment.
Pull request overview
This PR addresses GH-47481 by making record_batch_reader_source report an implicit (meaningful) ordering, allowing Acero’s fetch node (used for LIMIT/OFFSET) to accept it without requiring an explicit order_by.
Changes:
- Mark
RecordBatchReaderSourceNodeoutput ordering asOrdering::Implicit()sofetchno longer rejects it as non-deterministic.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
UT added. Thx @kou . |
zanmato1984
left a comment
There was a problem hiding this comment.
+1. I think this change aligns the ordering declaration of record_batch_reader_source with other similar sources (e.g. SchemaSourceNode). Thanks for working on this.
Rationale for this change
record_batch_reader_source does not support
select * limit kas described in #47481What changes are included in this PR?
record_batch_reader_source specify Ordering::Implicit
Are these changes tested?
yes
Are there any user-facing changes?
no
select * limit 3#47481