You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "ozankabak (via GitHub)" <gi...@apache.org> on 2023/11/03 21:13:44 UTC
[PR] Improve comments [arrow-datafusion]
ozankabak opened a new pull request, #8047:
URL: https://github.com/apache/arrow-datafusion/pull/8047
## Which issue does this PR close?
N/A.
## Rationale for this change
Improves comments for the `PartitionSearchMode` struct (make it clear it applies to both grouping and windowing).
## What changes are included in this PR?
Comment improvements.
## Are these changes tested?
Existing tests should pass.
## Are there any user-facing changes?
No.
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve comments for `PartitionSearchMode` struct [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #8047:
URL: https://github.com/apache/arrow-datafusion/pull/8047
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve comments for `PartitionSearchMode` struct [arrow-datafusion]
Posted by "ozankabak (via GitHub)" <gi...@apache.org>.
ozankabak commented on PR #8047:
URL: https://github.com/apache/arrow-datafusion/pull/8047#issuecomment-1793380626
BTW this is a first step towards improving comments and finding a better name/place for the `PartitionSearchMode` struct as discussed in https://github.com/apache/arrow-datafusion/pull/8006#discussion_r1380428109
@alamb, given that this struct simply encodes whether a given set of expressions have a partial or full permutation satisfying the ordering of a table, should we give it a more generic name like `OrderedPermutation`? If we do that, we probably would want to change the variant names to something like `None`, `Partial` and `Full` too. If we general-purposify this struct in this manner, where do you think this struct definition should be?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org