Re: [PATCH] Push limit to sort through a subquery

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Douglas Doole <dougdoole(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Konstantin Knizhnik <k(dot)knizhnik(at)postgrespro(dot)ru>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Push limit to sort through a subquery
Date: 2017-08-24 18:24:40
Message-ID: 6343.1503599080@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Douglas Doole <dougdoole(at)gmail(dot)com> writes:
>> TBH I dislike the fact that
>> you did the subquery case randomly differently from the existing cases;
>> it should just have been added as an additional recursive case. Since
>> this is done only once at query startup, worrying about hypothetical
>> micro-performance issues seems rather misguided.

> Habit mostly - why write code with potential performance problems when the
> alternative is just as easy to read?

I disagree that having adjacent code doing the same thing in two different
ways is "easy to read"; what it is is confusing. More globally, since
we're dealing with community code that will be read and hacked on by many
people, I think we need to prioritize simplicity, readability, and
extensibility over micro-efficiency. I'm willing to compromise those
goals for efficiency where a clear need has been demonstrated, but no such
need has been shown here, nor do I think it's likely that one can be
shown.

I'd have been okay with changing the entire function if it could still be
doing all cases the same way. But the exception for merge-append (and
probably soon other cases) means you still end up with a readability
problem.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2017-08-24 19:07:28 Re: Standby corruption after master is restarted
Previous Message Peter Eisentraut 2017-08-24 18:22:17 Re: SCRAM salt length