Thread: Re: DEALLOCATE IF EXISTS
On Tue, Oct 9, 2012 at 4:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I don't see Sébastien's message, but I made the same mistake in my patch. Another one is attached with copyfuncs and equalfuncs. I did a grep for DeallocateStmt and I don't believe I have missed anything else.
Also, I'm changing the subject so as not to hijack this thread any further.
Sébastien Lardière <slardiere@hi-media.com> writes:Still wrong ... at the very least you missed copyfuncs/equalfuncs.
> Indeed, brackets was not correct, it's better now (I think), and correct
> some comments.
In general, when adding a field to a struct, it's good practice to
grep for all uses of that struct.
I don't see Sébastien's message, but I made the same mistake in my patch. Another one is attached with copyfuncs and equalfuncs. I did a grep for DeallocateStmt and I don't believe I have missed anything else.
Also, I'm changing the subject so as not to hijack this thread any further.
Attachment
On Tue, Oct 9, 2012 at 4:44 PM, Vik Reykja <vikreykja@gmail.com> wrote:
On Tue, Oct 9, 2012 at 4:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:Sébastien Lardière <slardiere@hi-media.com> writes:Still wrong ... at the very least you missed copyfuncs/equalfuncs.
> Indeed, brackets was not correct, it's better now (I think), and correct
> some comments.
In general, when adding a field to a struct, it's good practice to
grep for all uses of that struct.
I don't see Sébastien's message, but I made the same mistake in my patch. Another one is attached with copyfuncs and equalfuncs. I did a grep for DeallocateStmt and I don't believe I have missed anything else.
Also, I'm changing the subject so as not to hijack this thread any further.
I am taking no comments to mean no objections and have added this to the next commitfest.
On Tue, 09 Oct 2012 16:44:07 +0200, Vik Reykja <vikreykja@gmail.com> wrote: > I don't see Sébastien's message, but I made the same mistake in my patch. > Another one is attached with copyfuncs and equalfuncs. I did a grep for > DeallocateStmt and I don't believe I have missed anything else. The patch looks pretty straightforward to me, except for one thing: previously there was no NOTICE if you sent a Close message to the server and the prepared statement didn't exist. But I'm leaving it for the committer to decide whether that's a problem or not, and marking this one Ready for Committer. Regards, Marko Tiikkaja
On 09.10.2012 17:44, Vik Reykja wrote: > On Tue, Oct 9, 2012 at 4:09 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote: > >> Sébastien Lardière<slardiere@hi-media.com> writes: >>> Indeed, brackets was not correct, it's better now (I think), and correct >>> some comments. >> >> Still wrong ... at the very least you missed copyfuncs/equalfuncs. >> In general, when adding a field to a struct, it's good practice to >> grep for all uses of that struct. >> > > I don't see Sébastien's message, but I made the same mistake in my patch. > Another one is attached with copyfuncs and equalfuncs. I did a grep for > DeallocateStmt and I don't believe I have missed anything else. > > Also, I'm changing the subject so as not to hijack this thread any further. I fail to see the point of DEALLOCATE IF EXISTS. Do you have real use case for this, or was this just a case of adding IF EXISTS to all commands for the sake of completeness? Usually the client knows what statements have been prepared, but perhaps you want to make sure everything is deallocated in some error handling case or similar. But in that case, you might as well just issue a regular DEALLOCATE and ignore errors. Or even more likely, you'll want to use DEALLOCATE ALL. - Heikki -- - Heikki
<br /><div class="gmail_extra"><div class="gmail_quote">On Tue, Nov 27, 2012 at 3:15 PM, Heikki Linnakangas <span dir="ltr"><<ahref="mailto:hlinnakangas@vmware.com" target="_blank">hlinnakangas@vmware.com</a>></span> wrote:<br /><blockquoteclass="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I fail to see thepoint of DEALLOCATE IF EXISTS. Do you have real use case for this, or was this just a case of adding IF EXISTS to allcommands for the sake of completeness?<br /><br /> Usually the client knows what statements have been prepared, but perhapsyou want to make sure everything is deallocated in some error handling case or similar. But in that case, you mightas well just issue a regular DEALLOCATE and ignore errors. Or even more likely, you'll want to use DEALLOCATE ALL.<spanclass="HOEnZb"><font color="#888888"></font></span><br /></blockquote></div><br />Hmm. The test case I had forit, which was very annoying in an "I want to be lazy" sort of way, I am unable to reproduce now. So I guess this becomesa "make it like the others" and the community can decide whether that's desirable.<br /><br />In my personal case,which again I can't reproduce because it's been a while since I've done it, DEALLOCATE ALL would have worked. I wasbasically preparing a query to work on it in the same conditions that it would be executed in a function, and I was onlyworking on one of these at a time so ALL would have been fine.<br /></div>
On 30.11.2012 12:05, Vik Reykja wrote: > On Tue, Nov 27, 2012 at 3:15 PM, Heikki Linnakangas<hlinnakangas@vmware.com >> wrote: > >> I fail to see the point of DEALLOCATE IF EXISTS. Do you have real use case >> for this, or was this just a case of adding IF EXISTS to all commands for >> the sake of completeness? >> >> Usually the client knows what statements have been prepared, but perhaps >> you want to make sure everything is deallocated in some error handling case >> or similar. But in that case, you might as well just issue a regular >> DEALLOCATE and ignore errors. Or even more likely, you'll want to use >> DEALLOCATE ALL. > > Hmm. The test case I had for it, which was very annoying in an "I want to > be lazy" sort of way, I am unable to reproduce now. So I guess this > becomes a "make it like the others" and the community can decide whether > that's desirable. > > In my personal case, which again I can't reproduce because it's been a > while since I've done it, DEALLOCATE ALL would have worked. I was > basically preparing a query to work on it in the same conditions that it > would be executed in a function, and I was only working on one of these at > a time so ALL would have been fine. Ok. Being the lazy person that I am, I'm going to just mark this as rejected then. There is no consensus that we should decorate every DDL command with "IF EXISTS", and even if we did, it's not clear that it should include DEALLOCATE. But thanks for the effort anyway! - Heikki
<br /><div class="gmail_extra"><div class="gmail_quote">On Wed, Dec 5, 2012 at 9:12 AM, Heikki Linnakangas <span dir="ltr"><<ahref="mailto:hlinnakangas@vmware.com" target="_blank">hlinnakangas@vmware.com</a>></span> wrote:<br /><blockquoteclass="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Ok. Being the lazyperson that I am, I'm going to just mark this as rejected then. There is no consensus that we should decorate every DDLcommand with "IF EXISTS", and even if we did, it's not clear that it should include DEALLOCATE. But thanks for the effortanyway!<span class="HOEnZb"><font color="#888888"></font></span><br /></blockquote></div><br />Fair enough. :-)<br/>Thanks for taking a look at it.<br /></div>