Thread: Re: [BUGS] BUG #6572: The example of SPI_execute is bogus
umi.tanuki@gmail.com writes: > http://www.postgresql.org/docs/9.1/static/spi-spi-execute.html > === > SPI_execute("INSERT INTO foo SELECT * FROM bar", false, 5); > will allow at most 5 rows to be inserted into the table. > === > This seems not true unless I'm missing something. Hmm ... that did work as described, until we broke it :-(. This is an oversight in the 9.0 changes that added separate ModifyTuple nodes to plan trees. ModifyTuple doesn't return after each updated row, unless there's a RETURNING clause; which means that the current_tuple_count check logic in ExecutePlan() no longer stops execution as intended. Given the lack of complaints since 9.0, maybe we should not fix this but just redefine the new behavior as being correct? But it seems mighty inconsistent that the tuple limit would apply if you have RETURNING but not when you don't. In any case, the ramifications are wider than one example in the SPI docs. Thoughts? regards, tom lane
On Wed, Apr 4, 2012 at 8:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > umi.tanuki@gmail.com writes: >> http://www.postgresql.org/docs/9.1/static/spi-spi-execute.html > >> === >> SPI_execute("INSERT INTO foo SELECT * FROM bar", false, 5); >> will allow at most 5 rows to be inserted into the table. >> === > >> This seems not true unless I'm missing something. > > Hmm ... that did work as described, until we broke it :-(. This is an > oversight in the 9.0 changes that added separate ModifyTuple nodes to > plan trees. ModifyTuple doesn't return after each updated row, unless > there's a RETURNING clause; which means that the current_tuple_count > check logic in ExecutePlan() no longer stops execution as intended. > > Given the lack of complaints since 9.0, maybe we should not fix this > but just redefine the new behavior as being correct? But it seems > mighty inconsistent that the tuple limit would apply if you have > RETURNING but not when you don't. In any case, the ramifications > are wider than one example in the SPI docs. > > Thoughts? To be honest, I was surprised when I found tcount parameter is said to be applied to even INSERT. I believe people think that parameter is to limit memory consumption when returning tuples thus it'd be applied for only SELECT or DML with RETURNING. So I'm +1 for non-fix but redefine the behavior. Who wants to limit the number of rows processed inside the backend, from SPI? Thanks, -- Hitoshi Harada
On Thu, Apr 5, 2012 at 2:39 AM, Hitoshi Harada <umi.tanuki@gmail.com> wrote: > On Wed, Apr 4, 2012 at 8:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> umi.tanuki@gmail.com writes: >>> http://www.postgresql.org/docs/9.1/static/spi-spi-execute.html >> >>> === >>> SPI_execute("INSERT INTO foo SELECT * FROM bar", false, 5); >>> will allow at most 5 rows to be inserted into the table. >>> === >> >>> This seems not true unless I'm missing something. >> >> Hmm ... that did work as described, until we broke it :-(. This is an >> oversight in the 9.0 changes that added separate ModifyTuple nodes to >> plan trees. ModifyTuple doesn't return after each updated row, unless >> there's a RETURNING clause; which means that the current_tuple_count >> check logic in ExecutePlan() no longer stops execution as intended. >> >> Given the lack of complaints since 9.0, maybe we should not fix this >> but just redefine the new behavior as being correct? But it seems >> mighty inconsistent that the tuple limit would apply if you have >> RETURNING but not when you don't. In any case, the ramifications >> are wider than one example in the SPI docs. >> >> Thoughts? > > To be honest, I was surprised when I found tcount parameter is said to > be applied to even INSERT. I believe people think that parameter is > to limit memory consumption when returning tuples thus it'd be applied > for only SELECT or DML with RETURNING. So I'm +1 for non-fix but > redefine the behavior. Who wants to limit the number of rows > processed inside the backend, from SPI? Yeah. I think it would be a good idea for UPDATE and DELETE to expose a LIMIT option, but I can't really see the virtue in making that functionality available only through SPI. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
> > Yeah. I think it would be a good idea for UPDATE and DELETE to expose > a LIMIT option, but I can't really see the virtue in making that > functionality available only through SPI. > I don't agree - LIMIT after UPDATE or DELETE has no sense. Clean solution should be based on using updateable CTE. Regards Pavel
On Fri, Apr 13, 2012 at 10:43 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> Yeah. I think it would be a good idea for UPDATE and DELETE to expose >> a LIMIT option, but I can't really see the virtue in making that >> functionality available only through SPI. > > I don't agree - LIMIT after UPDATE or DELETE has no sense. Clean > solution should be based on using updateable CTE. It has a lot of sense. Without it, it's very difficult to do logical replication on a table with no primary key. (Whether or not people should create such tables in the first place is, of course, beside the point.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
2012/4/14 Robert Haas <robertmhaas@gmail.com>: > On Fri, Apr 13, 2012 at 10:43 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >>> Yeah. I think it would be a good idea for UPDATE and DELETE to expose >>> a LIMIT option, but I can't really see the virtue in making that >>> functionality available only through SPI. >> >> I don't agree - LIMIT after UPDATE or DELETE has no sense. Clean >> solution should be based on using updateable CTE. > > It has a lot of sense. Without it, it's very difficult to do logical > replication on a table with no primary key. > > (Whether or not people should create such tables in the first place > is, of course, beside the point.) I am not against to functionality - I am against just to syntax DELETE FROM tab LIMIT x because is it ambiguous what means: DELETE FROM tab RETURNING * LIMIT x Regards Pavel > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company
On Sat, Apr 14, 2012 at 3:27 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> It has a lot of sense. Without it, it's very difficult to do logical >> replication on a table with no primary key. >> >> (Whether or not people should create such tables in the first place >> is, of course, beside the point.) > > I am not against to functionality - I am against just to syntax DELETE > FROM tab LIMIT x > > because is it ambiguous what means: DELETE FROM tab RETURNING * LIMIT x What's ambiguous about that? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On lör, 2012-04-14 at 08:23 -0400, Robert Haas wrote: > On Sat, Apr 14, 2012 at 3:27 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > >> It has a lot of sense. Without it, it's very difficult to do logical > >> replication on a table with no primary key. > >> > >> (Whether or not people should create such tables in the first place > >> is, of course, beside the point.) > > > > I am not against to functionality - I am against just to syntax DELETE > > FROM tab LIMIT x > > > > because is it ambiguous what means: DELETE FROM tab RETURNING * LIMIT x > > What's ambiguous about that? I suppose one could wonder whether the LIMIT applies to the deleting or just the returning.
2012/4/14 Peter Eisentraut <peter_e@gmx.net>: > On lör, 2012-04-14 at 08:23 -0400, Robert Haas wrote: >> On Sat, Apr 14, 2012 at 3:27 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> >> It has a lot of sense. Without it, it's very difficult to do logical >> >> replication on a table with no primary key. >> >> >> >> (Whether or not people should create such tables in the first place >> >> is, of course, beside the point.) >> > >> > I am not against to functionality - I am against just to syntax DELETE >> > FROM tab LIMIT x >> > >> > because is it ambiguous what means: DELETE FROM tab RETURNING * LIMIT x >> >> What's ambiguous about that? > > I suppose one could wonder whether the LIMIT applies to the deleting or > just the returning. > yes, exactly Regards Pavel
On Sat, Apr 14, 2012 at 12:15 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > On lör, 2012-04-14 at 08:23 -0400, Robert Haas wrote: >> On Sat, Apr 14, 2012 at 3:27 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >> >> It has a lot of sense. Without it, it's very difficult to do logical >> >> replication on a table with no primary key. >> >> >> >> (Whether or not people should create such tables in the first place >> >> is, of course, beside the point.) >> > >> > I am not against to functionality - I am against just to syntax DELETE >> > FROM tab LIMIT x >> > >> > because is it ambiguous what means: DELETE FROM tab RETURNING * LIMIT x >> >> What's ambiguous about that? > > I suppose one could wonder whether the LIMIT applies to the deleting or > just the returning. I suppose. I had in mind it would apply to both. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
2012-04-14 18:15 keltezéssel, Peter Eisentraut írta: > On lör, 2012-04-14 at 08:23 -0400, Robert Haas wrote: >> On Sat, Apr 14, 2012 at 3:27 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote: >>>> It has a lot of sense. Without it, it's very difficult to do logical >>>> replication on a table with no primary key. >>>> >>>> (Whether or not people should create such tables in the first place >>>> is, of course, beside the point.) >>> I am not against to functionality - I am against just to syntax DELETE >>> FROM tab LIMIT x >>> >>> because is it ambiguous what means: DELETE FROM tab RETURNING * LIMIT x >> What's ambiguous about that? > I suppose one could wonder whether the LIMIT applies to the deleting or > just the returning. Ambigous only in this order. LIMIT x RETURNING * wouldn't be. -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig& Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/
2012/4/15 Boszormenyi Zoltan <zb@cybertec.at>: > 2012-04-14 18:15 keltezéssel, Peter Eisentraut írta: > >> On lör, 2012-04-14 at 08:23 -0400, Robert Haas wrote: >>> >>> On Sat, Apr 14, 2012 at 3:27 AM, Pavel Stehule<pavel.stehule@gmail.com> >>> wrote: >>>>> >>>>> It has a lot of sense. Without it, it's very difficult to do logical >>>>> replication on a table with no primary key. >>>>> >>>>> (Whether or not people should create such tables in the first place >>>>> is, of course, beside the point.) >>>> >>>> I am not against to functionality - I am against just to syntax DELETE >>>> FROM tab LIMIT x >>>> >>>> because is it ambiguous what means: DELETE FROM tab RETURNING * LIMIT x >>> >>> What's ambiguous about that? >> >> I suppose one could wonder whether the LIMIT applies to the deleting or >> just the returning. > > > Ambigous only in this order. LIMIT x RETURNING * wouldn't be. but theoretically you can has two LIMIT clauses in one SQL statements DELETE FROM tab LIMIT n RETURNING * LIMIT m without updatable CTE it is probably only one solution, but because we have UCTE, then we don't need this construct. Regards Pavel > > -- > ---------------------------------- > Zoltán Böszörményi > Cybertec Schönig& Schönig GmbH > Gröhrmühlgasse 26 > A-2700 Wiener Neustadt, Austria > Web: http://www.postgresql-support.de > http://www.postgresql.at/ >
Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Apr 5, 2012 at 2:39 AM, Hitoshi Harada <umi.tanuki@gmail.com> wrote:
>> On Wed, Apr 4, 2012 at 8:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Given the lack of complaints since 9.0, maybe we should not fix this
>>> but just redefine the new behavior as being correct? But it seems
>>> mighty inconsistent that the tuple limit would apply if you have
>>> RETURNING but not when you don't. In any case, the ramifications
>>> are wider than one example in the SPI docs.
>> To be honest, I was surprised when I found tcount parameter is said to
>> be applied to even INSERT. I believe people think that parameter is
>> to limit memory consumption when returning tuples thus it'd be applied
>> for only SELECT or DML with RETURNING. So I'm +1 for non-fix but
>> redefine the behavior. Who wants to limit the number of rows
>> processed inside the backend, from SPI?
> Yeah.
Okay, apparently nobody cares about RETURNING behaving differently from
non-RETURNING, so the consensus is to redefine the current behavior as
correct. That means what we need is to go through the docs and see what
places need to be updated (and, I guess, back-patch the changes to 9.0).
I will get to this if nobody else does, but not right away.
> I think it would be a good idea for UPDATE and DELETE to expose
> a LIMIT option, but I can't really see the virtue in making that
> functionality available only through SPI.
FWIW, I'm not excited about that. You can get well-defined behavior
today from a SELECT/LIMIT drawing from a writable CTE (namely, that
the UPDATE/DELETE runs to completion but you only see a subset of
its RETURNING result). LIMIT directly on the UPDATE/DELETE would be
ill-defined, unless perhaps you want to also invent a way of specifying
the order in which rows get selected for update; but I don't want to
go there.
regards, tom lane
On Sun, Apr 15, 2012 at 12:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I think it would be a good idea for UPDATE and DELETE to expose >> a LIMIT option, but I can't really see the virtue in making that >> functionality available only through SPI. > > FWIW, I'm not excited about that. You can get well-defined behavior > today from a SELECT/LIMIT drawing from a writable CTE (namely, that > the UPDATE/DELETE runs to completion but you only see a subset of > its RETURNING result). LIMIT directly on the UPDATE/DELETE would be > ill-defined, unless perhaps you want to also invent a way of specifying > the order in which rows get selected for update; but I don't want to > go there. In the use cases I'm thinking of, it doesn't matter which row you decide to update or delete, only that you pick a single one. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Sun, Apr 15, 2012 at 12:29:39PM -0400, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > On Thu, Apr 5, 2012 at 2:39 AM, Hitoshi Harada <umi.tanuki@gmail.com> wrote: > >> On Wed, Apr 4, 2012 at 8:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >>> Given the lack of complaints since 9.0, maybe we should not fix this > >>> but just redefine the new behavior as being correct? But it seems > >>> mighty inconsistent that the tuple limit would apply if you have > >>> RETURNING but not when you don't. In any case, the ramifications > >>> are wider than one example in the SPI docs. > > >> To be honest, I was surprised when I found tcount parameter is said to > >> be applied to even INSERT. I believe people think that parameter is > >> to limit memory consumption when returning tuples thus it'd be applied > >> for only SELECT or DML with RETURNING. So I'm +1 for non-fix but > >> redefine the behavior. Who wants to limit the number of rows > >> processed inside the backend, from SPI? > > > Yeah. > > Okay, apparently nobody cares about RETURNING behaving differently from > non-RETURNING, so the consensus is to redefine the current behavior as > correct. That means what we need is to go through the docs and see what > places need to be updated (and, I guess, back-patch the changes to 9.0). > I will get to this if nobody else does, but not right away. Would someone make the doc change outlined above? Thanks. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
________________________________________ From: pgsql-bugs-owner@postgresql.org [pgsql-bugs-owner@postgresql.org] on behalf of Bruce Momjian [bruce@momjian.us] Sent: Wednesday, August 29, 2012 8:46 AM To: Tom Lane Cc: Robert Haas; Hitoshi Harada; pgsql-bugs@postgresql.org; pgsql-hackers@postgresql.org Subject: Re: [HACKERS] [BUGS] BUG #6572: The example of SPI_execute is bogus On Sun, Apr 15, 2012 at 12:29:39PM -0400, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > On Thu, Apr 5, 2012 at 2:39 AM, Hitoshi Harada <umi.tanuki@gmail.com> wrote: > >> On Wed, Apr 4, 2012 at 8:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >>> Given the lack of complaints since 9.0, maybe we should not fix this > >>> but just redefine the new behavior as being correct? But it seems > >>> mighty inconsistent that the tuple limit would apply if you have > >>> RETURNING but not when you don't. In any case, the ramifications > >>> are wider than one example in the SPI docs. > > >> To be honest, I was surprised when I found tcount parameter is said to > >> be applied to even INSERT. I believe people think that parameter is > >> to limit memory consumption when returning tuples thus it'd be applied > >> for only SELECT or DML with RETURNING. So I'm +1 for non-fix but > >> redefine the behavior. Who wants to limit the number of rows > >> processed inside the backend, from SPI? > > > Yeah. > > Okay, apparently nobody cares about RETURNING behaving differently from > non-RETURNING, so the consensus is to redefine the current behavior as > correct. That means what we need is to go through the docs and see what > places need to be updated (and, I guess, back-patch the changes to 9.0). > I will get to this if nobody else does, but not right away. > Would someone make the doc change outlined above? Thanks. I would like to work on this documentation bug. As per analysis I am planning to update following SPI function: 1. SPI_Execute: Here we will mention that argument count is used only for the kind of command which returns result i.e. allkind of SELECT and DML with returning clause. count is ignored for any other kind of commands. I will add one examplealso to indicate the difference. 2. SPI_execute_plan_with_paramlist: Here we can give just reference to SPI_execute i.e. I will mention that count has sameinterpretation as in SPI_execute. 3. SPI_execp: Here we can give just reference to SPI_execute i.e. I will mention that count has same interpretation as inSPI_execute. Please provide your feedback. Thanks and Regards, Kumar Rajeev Rastogi Cell No - +91 8971367787
On Wed, Aug 29, 2012 at 01:13:51PM +0000, Rajeev rastogi wrote: > ________________________________________ > From: pgsql-bugs-owner@postgresql.org [pgsql-bugs-owner@postgresql.org] on behalf of Bruce Momjian [bruce@momjian.us] > Sent: Wednesday, August 29, 2012 8:46 AM > To: Tom Lane > Cc: Robert Haas; Hitoshi Harada; pgsql-bugs@postgresql.org; pgsql-hackers@postgresql.org > Subject: Re: [HACKERS] [BUGS] BUG #6572: The example of SPI_execute is bogus > > On Sun, Apr 15, 2012 at 12:29:39PM -0400, Tom Lane wrote: > > Robert Haas <robertmhaas@gmail.com> writes: > > > On Thu, Apr 5, 2012 at 2:39 AM, Hitoshi Harada <umi.tanuki@gmail.com> wrote: > > >> On Wed, Apr 4, 2012 at 8:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > >>> Given the lack of complaints since 9.0, maybe we should not fix this > > >>> but just redefine the new behavior as being correct? But it seems > > >>> mighty inconsistent that the tuple limit would apply if you have > > >>> RETURNING but not when you don't. In any case, the ramifications > > >>> are wider than one example in the SPI docs. > > > > >> To be honest, I was surprised when I found tcount parameter is said to > > >> be applied to even INSERT. I believe people think that parameter is > > >> to limit memory consumption when returning tuples thus it'd be applied > > >> for only SELECT or DML with RETURNING. So I'm +1 for non-fix but > > >> redefine the behavior. Who wants to limit the number of rows > > >> processed inside the backend, from SPI? > > > > > Yeah. > > > > Okay, apparently nobody cares about RETURNING behaving differently from > > non-RETURNING, so the consensus is to redefine the current behavior as > > correct. That means what we need is to go through the docs and see what > > places need to be updated (and, I guess, back-patch the changes to 9.0). > > I will get to this if nobody else does, but not right away. > > > Would someone make the doc change outlined above? Thanks. > > > I would like to work on this documentation bug. > As per analysis I am planning to update following SPI function: > 1. SPI_Execute: Here we will mention that argument count is used only for the kind of command which returns result i.e.all kind of SELECT and DML with returning clause. count is ignored for any other kind of commands. I will add one examplealso to indicate the difference. > 2. SPI_execute_plan_with_paramlist: Here we can give just reference to SPI_execute i.e. I will mention that count has sameinterpretation as in SPI_execute. > 3. SPI_execp: Here we can give just reference to SPI_execute i.e. I will mention that count has same interpretation asin SPI_execute. Would someone please provide answers to these questions, or write a patch? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian <bruce@momjian.us> writes:
> Would someone make the doc change outlined above? Thanks.
Sorry, I'd nearly forgotten about this issue. Will see about fixing the
docs. (It looks like some of the comments in execMain.c could use work
too.)
regards, tom lane
On Thu, Jan 24, 2013 at 04:51:04PM -0500, Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Would someone make the doc change outlined above? Thanks. > > Sorry, I'd nearly forgotten about this issue. Will see about fixing the > docs. (It looks like some of the comments in execMain.c could use work > too.) Thanks. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +