On 05/12/2020 17:10, Andy Fan wrote:
Actually I can't understand this, could you explain more? Based on my
current
knowledge, when we run "SELECT DISTINCT a FROM t", we never care about
which operator to use for the unique.
SortGroupClause includes 'eqop' field, which determines the operator
that the expression needs to made unique with. The syntax doesn't let
you set it to anything else than the default btree opclass of the
datatype, though. But you can specify it for ORDER BY, and we use
SortGroupClauses to represent both sorting and grouping.
Also, if you use the same struct to also represent columns that you know
to be unique, and not just the DISTINCT clause in the query, then you
need the operator. For example, if you create a unique index on
non-default opfamily.
There's some precedence for PathKeys, as we generate PathKeys to
represent the DISTINCT column in PlannerInfo->distinct_pathkeys. On the
other hand, I've always found it confusing that we use PathKeys to
represent DISTINCT and GROUP BY, which are not actually sort orderings.
OK, I have the same confusion now:)
Perhaps it would make sense to store EquivalenceClass+opfamily in
UniqueKey, and also replace distinct_pathkeys and group_pathkeys with
UniqueKeys.
I can understand why we need EquivalenceClass for UniqueKey, but I can't
understand why we need opfamily here.
Thinking a bit harder, I guess we don't. Because EquivalenceClass
includes the operator family already, in the ec_opfamilies field.
For anyone who is interested with these patchsets, here is my plan
about this now. 1). I will try EquivalenceClass rather than Expr in
UniqueKey and add opfamily if needed. 2). I will start a new thread
to continue this topic. The current thread is too long which may
scare some people who may have interest in it. 3). I will give up
patch 5 & 6 for now. one reason I am not happy with the current
implementation, and the other reason is I want to make the patchset
smaller to make the reviewer easier. I will not give up them forever,
after the main part of this patchset is committed, I will continue
with them in a new thread. Thanks everyone for your input.
Sounds like a plan.
- Heikki
