On 05/13/2014 10:45 PM, Rukh Meski wrote:
On Sun, May 11, 2014 at 4:47 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
The $64 question is whether we'd accept an implementation that fails
if the target table has children (ie, is partitioned). That seems
to me to not be up to the project's usual quality expectations, but
maybe if there's enough demand for a partial solution we should do so.
It strikes me that a big part of the problem here is that the current
support for this case assumes that the children don't all have the
same rowtype. Which is important if you think of "child table" as
meaning "subclass in the OO sense". But for ordinary partitioning
cases it's just useless complexity, and ModifyTable isn't the only
thing that suffers from that useless complexity.
If we had a notion of "partitioned table" that involved a restriction
that all the child tables have the exact same rowtype, we could implement
UPDATE/DELETE in a much saner fashion --- just one plan tree, not one
per child table --- and it would be possible to support UPDATE/DELETE
ORDER BY LIMIT with no more work than for the single-table case.
So that might shift the calculation as to whether we're willing to
accept a partial implementation.
None of the use cases I have in mind would ever (have to) use this on
a parent table; in the worst case it might make sense to do it on the
child tables individually. Personally, I think that just refusing to
operate on tables with children is a reasonable start. I have no
interest in working on improving partitioning, but I don't think
pushing this feature back in the hopes that someone else will would
help anyone.
IMHO this needs to work with inheritance if we are to accept it. It
would be a rather strange limitation for no apparent reason, other than
that we didn't bother to implement it. It doesn't seem very difficult in
theory to add the table OID to the plan as a junk column, and use that
in the ModifyTable node to know which table a row came from.
In any case, the patch as it stands is clearly not acceptable, because
it just produces wrong results with inheritance. I'm marking it as
returned with feedback in the commitfest app. I'd suggest that you solve
the inheritance problems and resubmit.
Per the docs in the patch:
+ <para>
+ If the <literal>LIMIT</> (or <literal>FETCH FIRST</>) clause
+ is present, processing will stop after the system has attempted
+ to delete the specified amount of rows. In particular, if a row
+ was concurrently changed not to match the given <literal>WHERE</>
+ clause, it will count towards the <literal>LIMIT</> despite it
+ not being actually deleted. Unlike in <literal>SELECT</>, the
+ <literal>OFFSET</literal> clause is not available in
+ <literal>DELETE</>.
+ </para>
That behavior with READ COMMITTED mode and concurrent changes is
surprising. Do we really want it to behave like that, and if so, why?
Why is OFFSET not supported? Not that I care much for that, but I'm curious.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
