Re: LIMIT for UPDATE and DELETE

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