-
Notifications
You must be signed in to change notification settings - Fork 180
feat: support sort by #819
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
base: main
Are you sure you want to change the base?
Conversation
| ----------- | ------------------------------------------------------------ | ----------------------- | | ||
| Input | The relational input. | Required | | ||
| Sort Fields | List of one or more fields to sort by. Uses the same properties as the [orderedness](basics.md#orderedness) property. | One sort field required | | ||
| Preserve Partitioning | False means global sorting apply for entire data set, True means sorting only apply within the partition. | Optional | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not entirely against this but two things.
- This is physical stuff and we are mixing logical and physical concept. I am closer to purist (keeping logical and physical separate) but if you interpret "physical"
sort
onparticular distribution
, you don't need this field. If you want global sort, you can do it over serial (or singleton) partition. If you want distributed global sort, you can do it by running sort over global range distributed partition (i.e., sort on top of global range distributed). So as long as it does not change the distribution, this field does not matter. If you have a logical plan, well, the distribution does not exist so it is global sort. - Should this be
partition
ordistribution
? The existing documentation all referdistribution
so I suggest to usedistribution
for consistency.
So, if you can reason about the distribution, why do we need this new field? If this is even for the local parallelism (i.e., thread parallelism), you probably want to keep that orthogonal. So as long as sort does not and should not change any parallelism, distribution, I don't see a strong reason why we need this extra field.
A good example would be helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry,maybe this change is not the best solution, but i dont think it's physical stuff, in hive/spark, “select * from tb order by col” “select * from tb sort by col” have totally different meaning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I didn't know about SORT BY. I'd say it's still physical or exposing the implementation detail of execution because sort by
order is inherently tied to the number of reducers and the way the query result is constructed (i.e., what is the order of the sort by? without understanding of how the result is laid out, you can not define the order).
Anyway, I still think it is better to handled with proper distribution
and keep the distribution
and order
orthorgonal. In that regard, sort by
already assuming that you are generating some sort of distributed
result and you don't really care about the distribution
. I'm not sure whether that can be expressed in "logical" sense today (need to tinker on it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when we use sort by, we don't care about how the result is globally laid out. we won't use sort by in subquery, most time we use it with create table
. it's used to optimize output files. with this operator, we can make output files much smaller than unsorted files, and we can also easily build index on each partition. when we query this table next time, it's much faster than unsorted one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There you said create table
and partition
:-) The partition
is how the result is laid out, physically and globally, but the sort by
does not explicitly saying what those are -- just whatever assumed by the implementation.
As I said in the beginning, I'm not against the change -- modulo terms and how we represent this. The real distinction is whether the sort is a global sort (total order but still can be distributed) or not. I prefer local sort
for this (heavy-bias to the systems that I worked on).
many DBs support applying sort only within partition, not for entire data set. e.g. https://github.com/apache/spark/blob/1b41079f3f1e6cae12216c90d7820455222ee357/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala#L883