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.1399826...@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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to