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

From: Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com>
To: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] Erase the distinctClause if the result is unique by definition
Date: 2020-02-24 12:44:26
Message-ID: CAKU4AWpY6j4wW0rdX3xn+qisuydfV5fsE8bBnpZ-LA_UYohpTg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi All:

Here is the updated patch. It used some functions from
query_is_distinct_for.
I check the query's distinctness in create_distinct_paths, if it is
distinct already,
it will not generate the paths for that. so at last the query tree is not
untouched.

Please see if you have any comments. Thanks

On Mon, Feb 24, 2020 at 8:38 PM Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com> wrote:

>
>
> On Wed, Feb 12, 2020 at 12:36 AM Ashutosh Bapat <
> ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
>>
>>
>> On Tue, Feb 11, 2020 at 8:27 AM Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com>
>> wrote:
>>
>>>
>>>
>>> On Tue, Feb 11, 2020 at 12:22 AM Ashutosh Bapat <
>>> ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>>>
>>>>
>>>>
>>>>>
>>>>> [PATCH] Erase the distinctClause if the result is unique by
>>>>> definition
>>>>>
>>>>
>>>> I forgot to mention this in the last round of comments. Your patch was
>>>> actually removing distictClause from the Query structure. Please avoid
>>>> doing that. If you remove it, you are also removing the evidence that this
>>>> Query had a DISTINCT clause in it.
>>>>
>>>
>>> Yes, I removed it because it is the easiest way to do it. what is the
>>> purpose of keeping the evidence?
>>>
>>
>> Julien's example provides an explanation for this. The Query structure is
>> serialised into a view definition. Removing distinctClause from there means
>> that the view will never try to produce unique results.
>>
>>>
>>>
>>
> Actually it is not true. If a view is used in the query, the definition
> will be *copied*
> into the query tree. so if we modify the query tree, the definition of
> the view never
> touched. The issue of Julien reported is because of a typo error.
>
> -- session 2
>>> postgres=# alter table t alter column b drop not null;
>>> ALTER TABLE
>>>
>>> -- session 1:
>>> postgres=# explain execute st(1);
>>> QUERY PLAN
>>> -------------------------------------------------------------
>>> Unique (cost=1.03..1.04 rows=1 width=4)
>>> -> Sort (cost=1.03..1.04 rows=1 width=4)
>>> Sort Key: b
>>> -> Seq Scan on t (cost=0.00..1.02 rows=1 width=4)
>>> Filter: (c = $1)
>>> (5 rows)
>>>
>>
>> Since this prepared statement is parameterised PostgreSQL is replanning
>> it every time it gets executed. It's not using a stored prepared plan. Try
>> without parameters. Also make sure that a prepared plan is used for
>> execution and not a new plan.
>>
>
> Even for parameterised prepared statement, it is still possible to
> generate an generic
> plan. so it will not replanning every time. But no matter generic plan or
> not, after a DDL like
> changing the NOT NULL constraints. pg will generated a plan based on the
> stored query
> tree. However, the query tree will be *copied* again to generate a new
> plan. so even I
> modified the query tree, everything will be ok as well.
>
> At last, I am agreed with that modifying the query tree is not a good
> idea.
> so my updated patch doesn't use it any more.
>

Attachment Content-Type Size
0001-Erase-the-distinctClause-if-the-result-is-unique-by-.patch application/octet-stream 36.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dave Cramer 2020-02-24 13:09:52 Re: Error on failed COMMIT
Previous Message Hubert Zhang 2020-02-24 12:43:39 Re: Yet another vectorized engine