|From:||Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>|
|To:||Antonin Houska <ah(at)cybertec(dot)at>|
|Subject:||Re: [HACKERS] [PATCH] Incremental sort|
|Views:||Raw Message | Whole Thread | Download mbox|
On Mon, Jan 8, 2018 at 2:29 PM, Antonin Houska <ah(at)cybertec(dot)at> wrote:
> Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> > Antonin Houska <ah(at)cybertec(dot)at> wrote:
> > > Shouldn't the test contain *both* cases?
> > Thank you for pointing that. Sure, both cases are better. I've added
> second case as well as comments. Patch is attached.
> I'm fine with the tests now but have a minor comment on this comment:
> -- CROSS JOIN, not pushed down, because we don't push down LIMIT and
> remote side
> -- can't perform top-N sort like local side can.
> I think the note on LIMIT push-down makes the comment less clear because
> there's no difference in processing the LIMIT: EXPLAIN shows that both
> SELECT t1.c1, t2.c1 FROM ft1 t1 CROSS JOIN ft2 t2 ORDER BY t1.c1, t2.c1
> 100 LIMIT 10;
> SELECT t1.c3, t2.c3 FROM ft1 t1 CROSS JOIN ft2 t2 ORDER BY t1.c3, t2.c3
> 100 LIMIT 10;
> evaluate the LIMIT clause only locally.
> What I consider the important difference is that the 2nd case does not
> generate the appropriate input for remote incremental sort (while
> sort tends to be very cheap). Therefore it's cheaper to do no remote sort
> all and perform the top-N sort locally than to do a regular
> remote sort.
Agree, these comments are not clear enough. I've rewritten comments: they
more wordy, but now they look clearer for me. Also I've swapped the
queries order, for me
it seems to easier for understanding.
> I have no other questions about this patch. I expect the CFM to set the
> to "ready for committer" as soon as the other reviewers confirm they're
> about the patch status.
Good, thank you. Let's see what other reviewers will say.
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
|Next Message||Tom Lane||2018-01-08 19:18:11||Re: Parallel append plan instability/randomness|
|Previous Message||Fabien COELHO||2018-01-08 18:36:41||Re: pgbench - add \if support|