From: | James Coleman <jtc331(at)gmail(dot)com> |
---|---|
To: | Richard Guo <guofenglinux(at)gmail(dot)com> |
Cc: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: sqlsmith crash incremental sort |
Date: | 2020-04-16 16:04:03 |
Message-ID: | CAAaqYe_3xkgfcw9ZYOPcUCWUbod1yPKGm9wAY3XkJm7b2H+7jA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Apr 16, 2020 at 8:22 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
>
>
> On Thu, Apr 16, 2020 at 6:35 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
>>
>>
>> On Wed, Apr 15, 2020 at 10:47 PM Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> wrote:
>>>
>>>
>>> Well, yeah. The problem is the Unique simply compares the columns in the
>>> order it sees them, and it does not match the column order desired by
>>> incremental sort. But we don't push down this information at all :-(
>>
>>
>> This is a nice optimization better to have. Since the 'Sort and Unique'
>> would unique-ify the result of a UNION by sorting on all columns, why
>> not we adjust the sort order trying to match parse->sortClause so that
>> we can avoid the final sort node?
>>
>> Doing that we can transform plan from:
>>
>> # explain (costs off) select * from foo union select * from foo order by 1,3;
>> QUERY PLAN
>> -----------------------------------------------
>> Incremental Sort
>> Sort Key: foo.a, foo.c
>> Presorted Key: foo.a
>> -> Unique
>> -> Sort
>> Sort Key: foo.a, foo.b, foo.c
>> -> Append
>> -> Seq Scan on foo
>> -> Seq Scan on foo foo_1
>> (9 rows)
>>
>> To:
>>
>> # explain (costs off) select * from foo union select * from foo order by 1,3;
>> QUERY PLAN
>> -----------------------------------------
>> Unique
>> -> Sort
>> Sort Key: foo.a, foo.c, foo.b
>> -> Append
>> -> Seq Scan on foo
>> -> Seq Scan on foo foo_1
>> (6 rows)
>>
>
> Attached is what I'm thinking about this optimization. Does it make any
> sense?
Shouldn't this go one either a new thread or on the thread for the
patch Tomas was referencing (by Teodor I believe)?
Or are you saying you believe this patch guarantees we never see this
problem in incremental sort costing?
James
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2020-04-16 16:41:55 | fixing old_snapshot_threshold's time->xid mapping |
Previous Message | Mark Dilger | 2020-04-16 15:44:36 | Re: Should we add xid_current() or a int8->xid cast? |