On 8/29/14 12:20 PM, Etsuro Fujita wrote:
> The patch places limit-counting inside ModifyTable, and works well for
> inheritance trees, but I'm not sure that that is the right way to go. I
> think that this feature should be implemented in the way that we can
> naturally extend it to the ORDER-BY-LIMIT case in future. But honestly
> the patch doesn't seem to take into account that, I might be missing
> something, though.
The LIMIT part *has* to happen after the rows have been locked or it
will work very surprisingly under concurrency (sort of like how FOR
SHARE / FOR UPDATE worked before 9.0). So either it has to be inside
ModifyTable or the ModifyTable has to somehow pass something to a Limit
node on top of it which would then realize that the tuples from
ModifyTable aren't supposed to be sent to the client (unless there's a
RETURNING clause). I think it's a lot nicer to do the LIMITing inside
ModifyTable, even though that duplicates a small portion of code that
already exists in the Limit node.
> What plan do you have for the future extensibility?
>
> I think that the approach discussed in [1] would be promissing, so ISTM
> that it would be better to implement this feature by allowing for plans
> in the form of eg, ModifyTModifyTable+Limit+Append.
I don't see an approach discussed there, just a listing of problems with
no solutions.
This is just my personal opinion, but what I think should happen is:
1) We put the LIMIT inside ModifyTable like this patch does. This
doesn't prevent us from doing ORDER BY in the future, but helps numerous
people who today have to 2) We allow ORDER BY on tables with no inheritance children using
something similar to Rukh's previous patch. 3) Someone rewrites how UPDATE works based on Tom's suggestion here:
http://www.postgresql.org/message-id/1598.1399826841@sss.pgh.pa.us,
which allows us to support ORDER BY on all tables (or perhaps maybe not
FDWs, I don't know how those work). The LIMIT functionality in this
patch is unaffected.
Now, I know some people disagree with me on whether step #2 is worth
taking or not, but that's a separate discussion. My point w.r.t. this
patch still stands: I don't see any forwards compatibility problems with
this approach, nor do I really see any viable alternatives either.
.marko
