Thread: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

From
MLikharev@micropat.com
Date:
Hi,
I was asking this question some time ago and was under impression that this will be fixed in 8.x. In general problem
is,CREATE TEMP TABLE AS SELECT does not report any rows to the engine, seems like, so GET DIAGNOSTICS ROW_COUNT after
thestatement returns 0 as well as FOUND false. This was working in 7.3, but behavior changed in 7.4. 
My question is, will it be fixed or should I consider not stop using get diagnostic after ‘create temp table as select’
fromnow on. Honstly this was a very convinient feature especialy knowing that select count(*)  not a fastes possible
operation.

Thank you.



Re: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

From
Bruce Momjian
Date:
Can someone in the community comment on this question?  I don't know the
answer.

---------------------------------------------------------------------------

MLikharev@micropat.com wrote:
> Hi, I was asking this question some time ago and was under impression
> that this will be fixed in 8.x. In general problem is, CREATE TEMP
> TABLE AS SELECT does not report any rows to the engine, seems like, so
> GET DIAGNOSTICS ROW_COUNT after the statement returns 0 as well as
> FOUND false. This was working in 7.3, but behavior changed in 7.4.  My
> question is, will it be fixed or should I consider not stop using get
> diagnostic after ?create temp table as select? from now on. Honstly
> this was a very convinient feature especialy knowing that select
> count(*)  not a fastes possible operation.
>
> Thank you.
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
>  subscribe-nomail command to majordomo@postgresql.org so that your
>  message can get through to the mailing list cleanly
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Can someone in the community comment on this question?  I don't know the
> answer.

I think it could be changed back without much work, but I have a feeling
that we'd deliberately decided on the change of behavior.  Can anyone
recall a prior discussion, or want to vote with or against MLikharev?

Note that the change is actually at the SPI level, and would affect
SPI_processed for all code using CREATE AS/SELECT INTO through SPI,
not only plpgsql.

            regards, tom lane

Re: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

From
MLikharev@micropat.com
Date:
Hello,
I was not able to find any traces from the previous discussion trend,
but I believe that finished when I replaced GET DIAGNOSTIC with SELECT  COUNT().

Perfectly fine workaround, but more I look at that more I see why GET DIAGNOSTIC was so convenient not to mentioned
thatSELECT COUNT() is not a fastest  
possible statement in PG.

Ideally what I would like are:
1. “Official” word whether that will be supported or not, ether way is fine,
but that will clear confusion for me and others.
2. Maybe some clause in docs clarifying behavior for the case

Best regards.


Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Can someone in the community comment on this question?  I don't know the
> answer.

I think it could be changed back without much work, but I have a feeling
that we'd deliberately decided on the change of behavior.  Can anyone
recall a prior discussion, or want to vote with or against MLikharev?

Note that the change is actually at the SPI level, and would affect
SPI_processed for all code using CREATE AS/SELECT INTO through SPI,
not only plpgsql.

            regards, tom lane



Re: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

From
Bruce Momjian
Date:
I found a discussion of this issue from December, 2004:

    http://archives.postgresql.org/pgsql-general/2004-12/msg00070.php

The discussion trailed off with the idea that because no rows were
returned to the function, the row_count should be zero, but then there
was some discussion that FOUND was then inconsistent.

Anyway, perhaps we should read through this and make a final
determination.

---------------------------------------------------------------------------

MLikharev@micropat.com wrote:
> Hello,
> I was not able to find any traces from the previous discussion trend,
> but I believe that finished when I replaced GET DIAGNOSTIC with SELECT  COUNT().
>
> Perfectly fine workaround, but more I look at that more I see why GET DIAGNOSTIC was so convenient not to mentioned
thatSELECT COUNT() is not a fastest  
> possible statement in PG.
>
> Ideally what I would like are:
> 1. ?Official? word whether that will be supported or not, ether way is fine,
> but that will clear confusion for me and others.
> 2. Maybe some clause in docs clarifying behavior for the case
>
> Best regards.
>
>
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Can someone in the community comment on this question?  I don't know the
> > answer.
>
> I think it could be changed back without much work, but I have a feeling
> that we'd deliberately decided on the change of behavior.  Can anyone
> recall a prior discussion, or want to vote with or against MLikharev?
>
> Note that the change is actually at the SPI level, and would affect
> SPI_processed for all code using CREATE AS/SELECT INTO through SPI,
> not only plpgsql.
>
>             regards, tom lane
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
>                http://archives.postgresql.org
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I found a discussion of this issue from December, 2004:
>     http://archives.postgresql.org/pgsql-general/2004-12/msg00070.php

That was the same complainant ;-)

I dug through the CVS history and determined that the behavior changed
at spi.c rev 1.87:

2003-03-09 22:53  tgl

    * Restructure parsetree representation of
    DECLARE CURSOR: now it's a utility statement (DeclareCursorStmt)
    with a SELECT query dangling from it, rather than a SELECT query
    with a few unusual fields in it.  Add code to determine whether a
    planned query can safely be run backwards.  If DECLARE CURSOR
    specifies SCROLL, ensure that the plan can be run backwards by
    adding a Materialize plan node if it can't.  Without SCROLL, you
    get an error if you try to fetch backwards from a cursor that can't
    handle it.  (There is still some discussion about what the exact
    behavior should be, but this is necessary infrastructure in any
    case.) Along the way, make EXPLAIN DECLARE CURSOR work.

Looking at the code change, it may have just been a sloppy removal of a
local variable, ie checking queryDesc->dest rather than a previously
saved copy of same.  The log message certainly doesn't suggest that I
intended to change the behavior of CREATE TABLE AS.

So the initial evidence is that this was not an intentional change.
Do we want to revert it?  The behavior has been in the field now for
more than a full release cycle --- all 7.4.* releases behave this way
--- so one could argue that we should leave it be.

            regards, tom lane

Re: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

From
Alvaro Herrera
Date:
On Tue, May 31, 2005 at 01:03:30AM -0400, Tom Lane wrote:

> So the initial evidence is that this was not an intentional change.
> Do we want to revert it?  The behavior has been in the field now for
> more than a full release cycle --- all 7.4.* releases behave this way
> --- so one could argue that we should leave it be.

Well, probably nobody has complained because nobody uses the behavior.
I think it's wrong to assume that people really depend on this behavior
-- they would be doing something like

SELECT some_columns ...
CREATE TABLE AS SELECT ...
GET DIAGNOSTICS row_count

and expect to get the row_count from the first SELECT rather than CREATE
TABLE AS.  I wouldn't expect that, for one.  Personally I think it
should be reverted.

One thing: I couldn't find GET DIAGNOSTICS documentation for Oracle's
PL/SQL by googling around.  Is this PL/pgSQL-specific stuff?

--
Alvaro Herrera (<alvherre[a]surnet.cl>)
"Hay quien adquiere la mala costumbre de ser infeliz" (M. A. Evans)

Re: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

From
Tom Lane
Date:
Alvaro Herrera <alvherre@surnet.cl> writes:
> On Tue, May 31, 2005 at 01:03:30AM -0400, Tom Lane wrote:
>> So the initial evidence is that this was not an intentional change.
>> Do we want to revert it?  The behavior has been in the field now for
>> more than a full release cycle --- all 7.4.* releases behave this way
>> --- so one could argue that we should leave it be.

> ...  Personally I think it should be reverted.

OK, next question: is this a bug fix we should back-patch into 7.4,
or just change it in HEAD?

> One thing: I couldn't find GET DIAGNOSTICS documentation for Oracle's
> PL/SQL by googling around.  Is this PL/pgSQL-specific stuff?

It's in the SQL99 spec, though arguably plpgsql is misusing it.

            regards, tom lane

Re: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

From
Alvaro Herrera
Date:
On Tue, May 31, 2005 at 03:43:56PM -0400, Tom Lane wrote:
> Alvaro Herrera <alvherre@surnet.cl> writes:
> > On Tue, May 31, 2005 at 01:03:30AM -0400, Tom Lane wrote:
> >> So the initial evidence is that this was not an intentional change.
> >> Do we want to revert it?  The behavior has been in the field now for
> >> more than a full release cycle --- all 7.4.* releases behave this way
> >> --- so one could argue that we should leave it be.
>
> > ...  Personally I think it should be reverted.
>
> OK, next question: is this a bug fix we should back-patch into 7.4,
> or just change it in HEAD?

I guess apply only in HEAD, and provide the patch for MLikharev so he
can solve his immediate problem.

--
Alvaro Herrera (<alvherre[a]surnet.cl>)
Thou shalt study thy libraries and strive not to reinvent them without
cause, that thy code may be short and readable and thy days pleasant
and productive. (7th Commandment for C Programmers)

Re: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS

From
Neil Conway
Date:
On Tue, 2005-05-31 at 15:43 -0400, Tom Lane wrote:
> OK, next question: is this a bug fix we should back-patch into 7.4,
> or just change it in HEAD?

I agree with Alvaro: fix it in HEAD, but don't backport the change to
8.0 or 7.4.

-Neil



Re: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

From
Tom Lane
Date:
Alvaro Herrera <alvherre@surnet.cl> writes:
> On Tue, May 31, 2005 at 03:43:56PM -0400, Tom Lane wrote:
>> OK, next question: is this a bug fix we should back-patch into 7.4,
>> or just change it in HEAD?

> I guess apply only in HEAD, and provide the patch for MLikharev so he
> can solve his immediate problem.

Done.  Here is the patch (against CVS tip, but it should apply with
some fuzz in 8.0 or 7.4).

            regards, tom lane


*** src/backend/executor/spi.c.orig    Fri May  6 15:59:49 2005
--- src/backend/executor/spi.c    Thu Jun  9 16:59:37 2005
***************
*** 1497,1502 ****
--- 1497,1503 ----
  _SPI_pquery(QueryDesc *queryDesc, long tcount)
  {
      int            operation = queryDesc->operation;
+     CommandDest    origDest = queryDesc->dest->mydest;
      int            res;
      Oid            save_lastoid;

***************
*** 1548,1554 ****

      ExecutorEnd(queryDesc);

!     if (queryDesc->dest->mydest == SPI)
      {
          SPI_processed = _SPI_current->processed;
          SPI_lastoid = save_lastoid;
--- 1549,1556 ----

      ExecutorEnd(queryDesc);

!     /* Test origDest here so that SPI_processed gets set in SELINTO case */
!     if (origDest == SPI)
      {
          SPI_processed = _SPI_current->processed;
          SPI_lastoid = save_lastoid;

Re: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

From
MLikharev@micropat.com
Date:
Thanks.

Alvaro Herrera <alvherre@surnet.cl> writes:
> On Tue, May 31, 2005 at 03:43:56PM -0400, Tom Lane wrote:
>> OK, next question: is this a bug fix we should back-patch into 7.4,
>> or just change it in HEAD?

> I guess apply only in HEAD, and provide the patch for MLikharev so he
> can solve his immediate problem.

Done.  Here is the patch (against CVS tip, but it should apply with
some fuzz in 8.0 or 7.4).

            regards, tom lane



Re: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

From
"Ilja Golshtein"
Date:
Hi!

>Done.  Here is the patch (against CVS tip, but it should apply with
>some fuzz in 8.0 or 7.4).

Is this patch about CREATE TEMP TABLE AS SELECT only,
or about SELECT INTO TEMP TABLE as well?

--
Best regards
Ilja Golshtein

Re: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT

From
Bruce Momjian
Date:
Ilja Golshtein wrote:
> Hi!
>
> >Done.  Here is the patch (against CVS tip, but it should apply with
> >some fuzz in 8.0 or 7.4).
>
> Is this patch about CREATE TEMP TABLE AS SELECT only,
> or about SELECT INTO TEMP TABLE as well?

It should handle both because internally they are the same.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073