Re: [PATCH] Erase the distinctClause if the result is unique by definition

>
>
>> * There are some changes in existing regression cases that aren't
>> visibly related to the stated purpose of the patch, eg it now
>> notices that "select distinct max(unique2) from tenk1" doesn't
>> require an explicit DISTINCT step. That's not wrong, but I wonder
>> if maybe you should subdivide this patch into more than one patch,
>> because that must be coming from some separate change. I'm also
>> wondering what caused the plan change in expected/join.out.
>>
>
> Per my purpose it should be in the same patch, the logical here is we
> have distinct in the sql and the query is distinct already since the max
> function (the rule is defined in query_is_distinct_agg which is splited
> from
> the original query_is_distinct_for clause).
>

I think I was right until I come
into contrib/postgres_fdw/sql/postgres_fdw.sql.
Per my understanding, the query the result of "select max(a) from t" is
unique
since the aggregation function and has no group clause there. But in the
postgres_fdw.sql case, the Query->hasAggs is true for "select distinct
(select count(*) filter (where t2.c2 = 6 and t2.c1 < 10) from ft1 t1 where
t1.c1 = 6)
from ft2 t2 where t2.c2 % 6 = 0 order by 1;" This looks very strange to
me.
Is my understanding wrong or there is a bug here?

query->hasAggs was set to true in the following call stack.

pstate->p_hasAggs = true;

..

qry->hasAggs = pstate->p_hasAggs;

0 in check_agglevels_and_constraints of parse_agg.c:343
1 in transformAggregateCall of parse_agg.c:236
2 in ParseFuncOrColumn of parse_func.c:805
3 in transformFuncCall of parse_expr.c:1558
4 in transformExprRecurse of parse_expr.c:265
5 in transformExpr of parse_expr.c:155
6 in transformTargetEntry of parse_target.c:105
7 in transformTargetList of parse_target.c:193
8 in transformSelectStmt of analyze.c:1224
9 in transformStmt of analyze.c:301

You can see the new updated patch which should fix all the issues you point
out
except the one for supporting group by. The another reason for this patch
will
not be the final one is because the changes for postgres_fdw.out is too
arbitrary.
uploading it now just for reference. (The new introduced guc variable can
be
removed at last, keeping it now just make sure the testing is easier.)

At a high level, I'm a bit disturbed that this focuses only on DISTINCT
>>> and doesn't (appear to) have any equivalent intelligence for GROUP BY,
>>> though surely that offers much the same opportunities for optimization.
>>> It seems like it'd be worthwhile to take a couple steps back and see
>>> if we couldn't recast the logic to work for both.
>>>
>>>
>> OK, Looks group by is a bit harder than distinct since the aggregation
>> function.
>> I will go through the code to see where to add this logic.
>>
>>
>
> Can we grantee any_aggr_func(a) == a if only 1 row returned, if so, we
> can do
> some work on the pathtarget/reltarget by transforming the Aggref to raw
> expr.
> I checked the execution path of the aggregation call, looks it depends on
> Agg node
> which is the thing we want to remove.
>

We can't grantee any_aggr_func(a) == a when only 1 row returned, so the
above
method doesn't work. do you have any suggestion for this?