Horton

Thread

  1. [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Marti Raudsepp <marti@juffo.org> — 2010-11-28T15:14:53Z

    Hi list,

Often enough when developing PostgreSQL views and functions, I have
pasted the CREATE OR REPLACE commands into the wrong window/shell and
ran them there without realizing that I'm creating a function in the
wrong database, instead of replacing. Currently psql does not provide
any feedback of which action really occured.

Only after writing this patch I realized that I could instead raise a
NOTICE, like current IF EXISTS/IF NOT EXISTS clauses do. Is that a
better way to solve this?

This patch returns command tag "CREATE X" or "REPLACE X" for
LANGAUGE/VIEW/RULE/FUNCTION. This is done by passing completionTag to
from ProcessUtility to more functions, and adding a 'bool *didUpdate'
argument to some lower-level functions. I'm not sure if passing back
the status in a bool* is considered good style, but this way all the
functions look consistent.

Regards,
Marti

  2. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-11-28T16:29:02Z

    Marti Raudsepp <marti@juffo.org> writes:
> This patch returns command tag "CREATE X" or "REPLACE X" for
> LANGAUGE/VIEW/RULE/FUNCTION. This is done by passing completionTag to
> from ProcessUtility to more functions, and adding a 'bool *didUpdate'
> argument to some lower-level functions. I'm not sure if passing back
> the status in a bool* is considered good style, but this way all the
> functions look consistent.

This is going to break clients that expect commands to return the same
command tag as they have in the past.  I doubt that whatever usefulness
is gained will outweigh the compatibility problems.

			regards, tom lane

  3. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Robert Haas <robertmhaas@gmail.com> — 2010-11-29T00:18:14Z

    On Sun, Nov 28, 2010 at 11:29 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Marti Raudsepp <marti@juffo.org> writes:
>> This patch returns command tag "CREATE X" or "REPLACE X" for
>> LANGAUGE/VIEW/RULE/FUNCTION. This is done by passing completionTag to
>> from ProcessUtility to more functions, and adding a 'bool *didUpdate'
>> argument to some lower-level functions. I'm not sure if passing back
>> the status in a bool* is considered good style, but this way all the
>> functions look consistent.
>
> This is going to break clients that expect commands to return the same
> command tag as they have in the past.  I doubt that whatever usefulness
> is gained will outweigh the compatibility problems.

You complained about this when we changed the SELECT tag for 9.0 to
include row-counts for CTAS etc. where it hadn't before.  Have we
gotten any complaints about that change breaking clients?

I think more expessive command tags are in general a good thing.  The
idea that this particular change would be useful primarily for humans
examining the psql output seems a bit weak to me, but I can easily see
it being useful for programs.  Right now a program has no reason to
look at this command tag anyway; it'll always be the same.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  4. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-11-29T00:34:34Z

    Robert Haas <robertmhaas@gmail.com> writes:
> I think more expessive command tags are in general a good thing.  The
> idea that this particular change would be useful primarily for humans
> examining the psql output seems a bit weak to me, but I can easily see
> it being useful for programs.  Right now a program has no reason to
> look at this command tag anyway; it'll always be the same.

Hmm ... that's a good point, although I'm not sure that it's 100% true.

			regards, tom lane

  5. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Kouhei Kaigai <kaigai@ak.jp.nec.com> — 2011-01-14T04:35:19Z

    I tried to pick up this patch to review.

It seems to me fine, enough simple and works as explained in the
implementation level, apart from reasonability of this feature.
(Tom was not 100% agree with this feature 1.5month ago.)

I'm not certain whether the current regression test should be
updated, or not. The pg_regress launches psql with -q option,
so completionTag is always ignored.

Thanks,

(2010/11/29 0:14), Marti Raudsepp wrote:
> Hi list,
> 
> Often enough when developing PostgreSQL views and functions, I have
> pasted the CREATE OR REPLACE commands into the wrong window/shell and
> ran them there without realizing that I'm creating a function in the
> wrong database, instead of replacing. Currently psql does not provide
> any feedback of which action really occured.
> 
> Only after writing this patch I realized that I could instead raise a
> NOTICE, like current IF EXISTS/IF NOT EXISTS clauses do. Is that a
> better way to solve this?
> 
> This patch returns command tag "CREATE X" or "REPLACE X" for
> LANGAUGE/VIEW/RULE/FUNCTION. This is done by passing completionTag to
> from ProcessUtility to more functions, and adding a 'bool *didUpdate'
> argument to some lower-level functions. I'm not sure if passing back
> the status in a bool* is considered good style, but this way all the
> functions look consistent.
> 
> Regards,
> Marti
> 
-- 
KaiGai Kohei <kaigai@ak.jp.nec.com>

  6. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Robert Haas <robertmhaas@gmail.com> — 2011-01-14T11:40:07Z

    2011/1/13 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> I tried to pick up this patch to review.
>
> It seems to me fine, enough simple and works as explained in the
> implementation level, apart from reasonability of this feature.
> (Tom was not 100% agree with this feature 1.5month ago.)

Did you check whether this updated the code for 100% of the object
types where this could apply?

> I'm not certain whether the current regression test should be
> updated, or not. The pg_regress launches psql with -q option,
> so completionTag is always ignored.

Well, I don't see any easy way of regression testing it, then.  Am I
missing something?

Also, I don't really like the way this spreads knowledge of the
completionTag out all over the backend.  I think it would be better to
follow the existing model used by the COPY and COMMIT commands,
whereby the return value indicates what happened and
standard_ProcessUtility() uses that to set the command tag.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  7. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Alvaro Herrera <alvherre@commandprompt.com> — 2011-01-14T14:24:55Z

    Excerpts from Robert Haas's message of vie ene 14 08:40:07 -0300 2011:

> Also, I don't really like the way this spreads knowledge of the
> completionTag out all over the backend.  I think it would be better to
> follow the existing model used by the COPY and COMMIT commands,
> whereby the return value indicates what happened and
> standard_ProcessUtility() uses that to set the command tag.

Yeah, that looks ugly.  However it's already ugly elsewhere: for example
see PerformPortalFetch.  I am not sure if it should be this patch's
responsability to clean that stuff up.  (Maybe we should decree that at
least this patch shouldn't make the situation worse.)

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

  8. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Robert Haas <robertmhaas@gmail.com> — 2011-01-14T14:30:00Z

    On Fri, Jan 14, 2011 at 9:24 AM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:
> Excerpts from Robert Haas's message of vie ene 14 08:40:07 -0300 2011:
>
>> Also, I don't really like the way this spreads knowledge of the
>> completionTag out all over the backend.  I think it would be better to
>> follow the existing model used by the COPY and COMMIT commands,
>> whereby the return value indicates what happened and
>> standard_ProcessUtility() uses that to set the command tag.
>
> Yeah, that looks ugly.  However it's already ugly elsewhere: for example
> see PerformPortalFetch.  I am not sure if it should be this patch's
> responsability to clean that stuff up.  (Maybe we should decree that at
> least this patch shouldn't make the situation worse.)

Agreed: it's not the patch's job to clean it up, but it shouldn't make
the situation worse.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  9. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-01-14T17:07:14Z

    Alvaro Herrera <alvherre@commandprompt.com> writes:
> Excerpts from Robert Haas's message of vie ene 14 08:40:07 -0300 2011:
>> Also, I don't really like the way this spreads knowledge of the
>> completionTag out all over the backend.  I think it would be better to
>> follow the existing model used by the COPY and COMMIT commands,
>> whereby the return value indicates what happened and
>> standard_ProcessUtility() uses that to set the command tag.

> Yeah, that looks ugly.  However it's already ugly elsewhere: for example
> see PerformPortalFetch.  I am not sure if it should be this patch's
> responsability to clean that stuff up.  (Maybe we should decree that at
> least this patch shouldn't make the situation worse.)

I thought we were going to reject the patch outright anyway.  The
compatibility consequences of changing command tags are not worth the
benefit, independently of how ugly the backend-side code may or may
not be.

			regards, tom lane

  10. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Simon Riggs <simon@2ndquadrant.com> — 2011-01-14T17:15:56Z

    On Fri, 2011-01-14 at 12:07 -0500, Tom Lane wrote:

> I thought we were going to reject the patch outright anyway.  The
> compatibility consequences of changing command tags are not worth the
> benefit, independently of how ugly the backend-side code may or may
> not be.

+1

-- 
 Simon Riggs           http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services

  11. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Robert Haas <robertmhaas@gmail.com> — 2011-01-14T18:14:02Z

    On Fri, Jan 14, 2011 at 12:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
>> Excerpts from Robert Haas's message of vie ene 14 08:40:07 -0300 2011:
>>> Also, I don't really like the way this spreads knowledge of the
>>> completionTag out all over the backend.  I think it would be better to
>>> follow the existing model used by the COPY and COMMIT commands,
>>> whereby the return value indicates what happened and
>>> standard_ProcessUtility() uses that to set the command tag.
>
>> Yeah, that looks ugly.  However it's already ugly elsewhere: for example
>> see PerformPortalFetch.  I am not sure if it should be this patch's
>> responsability to clean that stuff up.  (Maybe we should decree that at
>> least this patch shouldn't make the situation worse.)
>
> I thought we were going to reject the patch outright anyway.  The
> compatibility consequences of changing command tags are not worth the
> benefit, independently of how ugly the backend-side code may or may
> not be.

My previous response to this criticism was here:

http://archives.postgresql.org/pgsql-hackers/2010-11/msg01899.php

Your response, which seemed at least partially in agreement, is here:

http://archives.postgresql.org/pgsql-hackers/2010-11/msg01901.php

If we're going to reject this patch on backwards-compatibility
grounds, we need to make an argument that the backward-compatibility
hazards are a real concern.  So, again, has anyone complained about
the changes we made in this area in 9.0?  And under what circumstances
do we foresee someone relying on the command tag of a command that
always returns the same tag?  I'm as quick as anyone to bow before a
compelling argument, but I don't think anyone's made such an argument.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  12. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-01-14T18:35:12Z

    Robert Haas <robertmhaas@gmail.com> writes:
> If we're going to reject this patch on backwards-compatibility
> grounds, we need to make an argument that the backward-compatibility
> hazards are a real concern.  So, again, has anyone complained about
> the changes we made in this area in 9.0?

That 9.0 change was far less invasive than this: it only added a count
field to SELECT and CTAS result tags.  Quite aside from the fact that
the tag name stayed the same, in the SELECT case it's unlikely anyone
would have checked the tag at all rather than just testing for
PQresultStatus() == PGRES_TUPLES_OK.  So it was basically only changing
the result for *one* command type.  I don't think it's a good basis for
arguing that this patch won't cause problems.

			regards, tom lane

  13. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Robert Haas <robertmhaas@gmail.com> — 2011-01-14T18:43:45Z

    On Fri, Jan 14, 2011 at 1:35 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> If we're going to reject this patch on backwards-compatibility
>> grounds, we need to make an argument that the backward-compatibility
>> hazards are a real concern.  So, again, has anyone complained about
>> the changes we made in this area in 9.0?
>
> That 9.0 change was far less invasive than this: it only added a count
> field to SELECT and CTAS result tags.  Quite aside from the fact that
> the tag name stayed the same, in the SELECT case it's unlikely anyone
> would have checked the tag at all rather than just testing for
> PQresultStatus() == PGRES_TUPLES_OK.  So it was basically only changing
> the result for *one* command type.  I don't think it's a good basis for
> arguing that this patch won't cause problems.

Yeah, but that one command tag was SELECT.  That's a pretty commonly
used command.  Most production environments probably use all of the
commands affected by this patch together an order of magnitude less
often than they use SELECT.

Again, on what basis are we arguing that people are going to be
looking at the command tag of a command that always returns the same
tag?  That seems pretty darn unlikely to me.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  14. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-01-14T19:07:19Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Jan 14, 2011 at 1:35 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> That 9.0 change was far less invasive than this: it only added a count
>> field to SELECT and CTAS result tags. Quite aside from the fact that
>> the tag name stayed the same, in the SELECT case it's unlikely anyone
>> would have checked the tag at all rather than just testing for
>> PQresultStatus() == PGRES_TUPLES_OK.

> Yeah, but that one command tag was SELECT.  That's a pretty commonly
> used command.

You're ignoring the point that people would probably use PQresultStatus
in preference to checking the tag at all, when dealing with SELECT.
psql itself is an example --- it never looks at the tag, nor shows it to
the user, in the SELECT case.  That patch only really changed the
exposed behavior for CREATE TABLE AS SELECT / SELECT INTO, neither of
which can be claimed to be hugely popular things for programs to issue.

The other side of the argument that needs to be considered is what the
benefit is.  There was a fairly clear functional gain from reporting
the rowcount for CTAS.  I'm less convinced that sending back REPLACE
is a big benefit worth taking big compatibility risks for.

			regards, tom lane

  15. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Robert Haas <robertmhaas@gmail.com> — 2011-01-14T19:44:39Z

    On Fri, Jan 14, 2011 at 2:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Fri, Jan 14, 2011 at 1:35 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> That 9.0 change was far less invasive than this: it only added a count
>>> field to SELECT and CTAS result tags.  Quite aside from the fact that
>>> the tag name stayed the same, in the SELECT case it's unlikely anyone
>>> would have checked the tag at all rather than just testing for
>>> PQresultStatus() == PGRES_TUPLES_OK.
>
>> Yeah, but that one command tag was SELECT.  That's a pretty commonly
>> used command.
>
> You're ignoring the point that people would probably use PQresultStatus
> in preference to checking the tag at all, when dealing with SELECT.

I would assume they would also use PQresultStatus() when checking
whether CREATE OR REPLACE FUNCTION worked.  Even if they were using
PQcmdStatus() for some reason, which seems like an odd thing to do,
there'd be no reason to check for anything beyond "is it empty?".  The
idea that there are massive amounts of code out there that are
expecting the command tag to be *exactly* CREATE FUNCTION and will
break if it differs by a byte seems quite improbable.  Can you produce
an example of any such code?

> The other side of the argument that needs to be considered is what the
> benefit is.  There was a fairly clear functional gain from reporting
> the rowcount for CTAS.  I'm less convinced that sending back REPLACE
> is a big benefit worth taking big compatibility risks for.

Asserting that there are "big compatibility risks" doesn't make it so
- you've offered no evidence of that.  Even if a handful of people had
complained about that one, I would still felt it was a good change,
but it doesn't seem that there are any at all.  I classify this as one
of a dozen or two minor usability enhancements that we make in every
release, and most people don't care, and those who do go "oh, that's
handy".  I think before we reject a patch for breaking things, we
ought to be able to identify either some actual application that is
broken by it, or at least some reasonably plausible coding pattern
that would blow up.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  16. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Marti Raudsepp <marti@juffo.org> — 2011-01-14T20:45:06Z

    Thanks for reviewing!

On Fri, Jan 14, 2011 at 13:40, Robert Haas <robertmhaas@gmail.com> wrote:
> Did you check whether this updated the code for 100% of the object
> types where this could apply?

I walked through all the CREATE statements in the documentation and
these four seem to be the only ones that accept FOR REPLACE.

There's a similar case with CREATE TABLE IF NOT EXISTS, maybe this is
worth covering in an updated patch too?
And if I change that, people might expect the same from DROP X IF EXISTS too?

> Also, I don't really like the way this spreads knowledge of the
> completionTag out all over the backend.  I think it would be better to
> follow the existing model used by the COPY and COMMIT commands,
> whereby the return value indicates what happened and
> standard_ProcessUtility() uses that to set the command tag.

Right. I created this pattern after PerformPortalFetch() which already
took a completionTag argument. But your approach seems more
reasonable.

Regards,
Marti

  17. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Robert Haas <robertmhaas@gmail.com> — 2011-01-14T23:45:57Z

    On Fri, Jan 14, 2011 at 3:45 PM, Marti Raudsepp <marti@juffo.org> wrote:
> There's a similar case with CREATE TABLE IF NOT EXISTS, maybe this is
> worth covering in an updated patch too?
> And if I change that, people might expect the same from DROP X IF EXISTS too?

It's far less clear what you'd change those cases to say, and they
already emit a NOTICE, so it seems unnecessary.

>> Also, I don't really like the way this spreads knowledge of the
>> completionTag out all over the backend.  I think it would be better to
>> follow the existing model used by the COPY and COMMIT commands,
>> whereby the return value indicates what happened and
>> standard_ProcessUtility() uses that to set the command tag.
>
> Right. I created this pattern after PerformPortalFetch() which already
> took a completionTag argument. But your approach seems more
> reasonable.

OK.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  18. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Marti Raudsepp <marti@juffo.org> — 2011-01-15T21:25:56Z

    Here's an updated patch that reports command status back to
ProcessUtility via 'bool' return value.

I was a bit unsure about using bool return values because it's not
immediately obvious what "true" or "false" refer to, but defining a
new enum seemed like overkill, so I went with bool anyway. Any better
ideas?

The 2nd patch also moves MOVE/FETCH command tag formatting up to
ProcessUtility, hopefully this change is for the better.

Regards,
Marti

  19. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Peter Eisentraut <peter_e@gmx.net> — 2011-01-17T14:41:42Z

    On fre, 2011-01-14 at 18:45 -0500, Robert Haas wrote:
> On Fri, Jan 14, 2011 at 3:45 PM, Marti Raudsepp <marti@juffo.org>
> wrote:
> > There's a similar case with CREATE TABLE IF NOT EXISTS, maybe this
> is
> > worth covering in an updated patch too?
> > And if I change that, people might expect the same from DROP X IF
> EXISTS too?
> 
> It's far less clear what you'd change those cases to say, and they
> already emit a NOTICE, so it seems unnecessary.

Maybe instead of the proposed patch, a notice could be added:

NOTICE: existing object was replaced

  20. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Robert Haas <robertmhaas@gmail.com> — 2011-01-17T15:05:31Z

    On Mon, Jan 17, 2011 at 9:41 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> On fre, 2011-01-14 at 18:45 -0500, Robert Haas wrote:
>> On Fri, Jan 14, 2011 at 3:45 PM, Marti Raudsepp <marti@juffo.org>
>> wrote:
>> > There's a similar case with CREATE TABLE IF NOT EXISTS, maybe this
>> is
>> > worth covering in an updated patch too?
>> > And if I change that, people might expect the same from DROP X IF
>> EXISTS too?
>>
>> It's far less clear what you'd change those cases to say, and they
>> already emit a NOTICE, so it seems unnecessary.
>
> Maybe instead of the proposed patch, a notice could be added:
>
> NOTICE: existing object was replaced

Well, that would eliminate the backward-compatibility hazard, pretty
much, but it seems noisy.  I already find some of these notices to be
unduly informative.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  21. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Peter Eisentraut <peter_e@gmx.net> — 2011-01-17T19:23:07Z

    On mån, 2011-01-17 at 10:05 -0500, Robert Haas wrote:
> On Mon, Jan 17, 2011 at 9:41 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> > On fre, 2011-01-14 at 18:45 -0500, Robert Haas wrote:
> >> On Fri, Jan 14, 2011 at 3:45 PM, Marti Raudsepp <marti@juffo.org>
> >> wrote:
> >> > There's a similar case with CREATE TABLE IF NOT EXISTS, maybe this
> >> is
> >> > worth covering in an updated patch too?
> >> > And if I change that, people might expect the same from DROP X IF
> >> EXISTS too?
> >>
> >> It's far less clear what you'd change those cases to say, and they
> >> already emit a NOTICE, so it seems unnecessary.
> >
> > Maybe instead of the proposed patch, a notice could be added:
> >
> > NOTICE: existing object was replaced
> 
> Well, that would eliminate the backward-compatibility hazard, pretty
> much, but it seems noisy.  I already find some of these notices to be
> unduly informative.

I'm also anti-NOTICE.

I'm just saying, we propose that CREATE OR REPLACE should return a tag
of CREATE or REPLACE depending on what it did, then DROP IF NOT EXISTS
should also return a tag of DROP or ??? depending on what it did.  Since
the latter question was settled by a notice, that would also be the
proper answer for the former.

Perhaps the next thing is that MERGE should return INSERT or UPDATE
depending on the outcome?

  22. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-01-17T19:25:44Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Jan 17, 2011 at 9:41 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
>> Maybe instead of the proposed patch, a notice could be added:
>> NOTICE: existing object was replaced

> Well, that would eliminate the backward-compatibility hazard, pretty
> much, but it seems noisy.  I already find some of these notices to be
> unduly informative.

ROTFL ...

There has been some previous banter about reorganizing or reclassifying
the various NOTICE messages to make them more useful and/or less noisy;
but I don't think we've ever had a really concrete proposal for better
behavior.  Maybe it's time to reopen that discussion.

I do agree with Peter's underlying point: it would be pretty
inconsistent for CREATE OR REPLACE to report this bit of info via
command tag when CREATE IF NOT EXISTS is reporting an absolutely
equivalent bit of info via elog(NOTICE).

			regards, tom lane

  23. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    David Fetter <david@fetter.org> — 2011-01-17T19:35:23Z

    On Mon, Jan 17, 2011 at 09:23:07PM +0200, Peter Eisentraut wrote:
> On mån, 2011-01-17 at 10:05 -0500, Robert Haas wrote:
> > On Mon, Jan 17, 2011 at 9:41 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> > > On fre, 2011-01-14 at 18:45 -0500, Robert Haas wrote:
> > >> On Fri, Jan 14, 2011 at 3:45 PM, Marti Raudsepp <marti@juffo.org>
> > >> wrote:
> > >> > There's a similar case with CREATE TABLE IF NOT EXISTS, maybe this
> > >> is
> > >> > worth covering in an updated patch too?
> > >> > And if I change that, people might expect the same from DROP X IF
> > >> EXISTS too?
> > >>
> > >> It's far less clear what you'd change those cases to say, and they
> > >> already emit a NOTICE, so it seems unnecessary.
> > >
> > > Maybe instead of the proposed patch, a notice could be added:
> > >
> > > NOTICE: existing object was replaced
> > 
> > Well, that would eliminate the backward-compatibility hazard, pretty
> > much, but it seems noisy.  I already find some of these notices to be
> > unduly informative.
> 
> I'm also anti-NOTICE.
> 
> I'm just saying, we propose that CREATE OR REPLACE should return a tag
> of CREATE or REPLACE depending on what it did, then DROP IF NOT EXISTS
> should also return a tag of DROP or ??? depending on what it did.  Since
> the latter question was settled by a notice, that would also be the
> proper answer for the former.
> 
> Perhaps the next thing is that MERGE should return INSERT or UPDATE
> depending on the outcome?

Given that it can do both in a single statement, I'm guessing that
this is intended to be facetious.  Or are you suggesting that the
command tags become an array?  This has all kinds of interesting
possibilities, but would of course break all kinds of stuff in the
process.

Cheers,
David.
-- 
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  24. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Robert Haas <robertmhaas@gmail.com> — 2011-01-17T19:52:33Z

    On Mon, Jan 17, 2011 at 2:25 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Mon, Jan 17, 2011 at 9:41 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
>>> Maybe instead of the proposed patch, a notice could be added:
>>> NOTICE: existing object was replaced
>
>> Well, that would eliminate the backward-compatibility hazard, pretty
>> much, but it seems noisy.  I already find some of these notices to be
>> unduly informative.
>
> ROTFL ...
>
> There has been some previous banter about reorganizing or reclassifying
> the various NOTICE messages to make them more useful and/or less noisy;
> but I don't think we've ever had a really concrete proposal for better
> behavior.  Maybe it's time to reopen that discussion.
>
> I do agree with Peter's underlying point: it would be pretty
> inconsistent for CREATE OR REPLACE to report this bit of info via
> command tag when CREATE IF NOT EXISTS is reporting an absolutely
> equivalent bit of info via elog(NOTICE).

There's a fine line between serious discussion, humor, and outright
mockery here, and I'm not too sure which one Peter's currently engaged
in.  I guess the point here for me is that commands tags for SELECT,
INSERT, UPDATE, and DELETE all return some useful information about
what actually happened - especially, a row count.  If it's reasonable
for those commands to return a row count in the command tag, then
there's no reason why utility commands shouldn't also be allowed to
return high-level status information as part of the command tag.  On
the flip side we could just rip out command tags altogether and have
psql print out the first two words of the input string.

The asymmetry between DROP-IF-EXISTS and CREATE-IF-NOT-EXISTS and the
proposed CREATE-OR-REPLACE behavior doesn't bother me very much,
because it's already asymmetric: the first two currently report what
happened, and the third one currently doesn't.  If you want to propose
to make all of them consistent, how?  I don't particularly like the
idea of adding a NOTICE here; we've got too many of those already[1].
Making DIE report that it didn't do anything via a command tag clearly
won't work, because you can say "DROP IF EXISTS foo, bar, baz" and the
answer might not be the same in all three cases, but COR has no such
issue.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[1] rhaas=# create table foo (a serial primary key);
NOTICE:  CREATE TABLE will create implicit sequence "foo_a_seq" for
serial column "foo.a"
NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index
"foo_pkey" for table "foo"
CREATE TABLE

Well, yeah, why did I say primary key if I didn't want a primary key
index to be created, and why did I say serial if I didn't want a
sequence?

  25. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Robert Haas <robertmhaas@gmail.com> — 2011-01-23T02:11:27Z

    On Mon, Jan 17, 2011 at 2:52 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Jan 17, 2011 at 2:25 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> On Mon, Jan 17, 2011 at 9:41 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
>>>> Maybe instead of the proposed patch, a notice could be added:
>>>> NOTICE: existing object was replaced
>>
>>> Well, that would eliminate the backward-compatibility hazard, pretty
>>> much, but it seems noisy.  I already find some of these notices to be
>>> unduly informative.
>>
>> ROTFL ...
>>
>> There has been some previous banter about reorganizing or reclassifying
>> the various NOTICE messages to make them more useful and/or less noisy;
>> but I don't think we've ever had a really concrete proposal for better
>> behavior.  Maybe it's time to reopen that discussion.
>>
>> I do agree with Peter's underlying point: it would be pretty
>> inconsistent for CREATE OR REPLACE to report this bit of info via
>> command tag when CREATE IF NOT EXISTS is reporting an absolutely
>> equivalent bit of info via elog(NOTICE).
>
> There's a fine line between serious discussion, humor, and outright
> mockery here, and I'm not too sure which one Peter's currently engaged
> in.  I guess the point here for me is that commands tags for SELECT,
> INSERT, UPDATE, and DELETE all return some useful information about
> what actually happened - especially, a row count.  If it's reasonable
> for those commands to return a row count in the command tag, then
> there's no reason why utility commands shouldn't also be allowed to
> return high-level status information as part of the command tag.  On
> the flip side we could just rip out command tags altogether and have
> psql print out the first two words of the input string.
>
> The asymmetry between DROP-IF-EXISTS and CREATE-IF-NOT-EXISTS and the
> proposed CREATE-OR-REPLACE behavior doesn't bother me very much,
> because it's already asymmetric: the first two currently report what
> happened, and the third one currently doesn't.  If you want to propose
> to make all of them consistent, how?  I don't particularly like the
> idea of adding a NOTICE here; we've got too many of those already[1].
> Making DIE report that it didn't do anything via a command tag clearly
> won't work, because you can say "DROP IF EXISTS foo, bar, baz" and the
> answer might not be the same in all three cases, but COR has no such
> issue.

It seems no one wants to put any further effort into this problem.  Bummer.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  26. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Robert Haas <robertmhaas@gmail.com> — 2011-01-28T19:04:08Z

    On Sat, Jan 22, 2011 at 9:11 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> It seems no one wants to put any further effort into this problem.  Bummer.

Since no one has felt the need to dispute the above statement in the
last 6 days, it seems clear to mark this Returned with Feedback, which
I have now done.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  27. Re: [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

    Marti Raudsepp <marti@juffo.org> — 2011-01-28T20:48:54Z

    On Fri, Jan 28, 2011 at 21:04, Robert Haas <robertmhaas@gmail.com> wrote:
> On Sat, Jan 22, 2011 at 9:11 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> It seems no one wants to put any further effort into this problem.  Bummer.
>
> Since no one has felt the need to dispute the above statement in the
> last 6 days, it seems clear to mark this Returned with Feedback, which
> I have now done.

Just to be clear, I could put more effort into this, but I agree with
your concerns about this introducing inconsistency. Given how few
people are interested in this change, it's probably best to drop it.

Regards,
Marti