Обсуждение: CREATE TEMP TABLE AS SELECT/ GET DIAGNOSTICS ROW_COUNT
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.
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
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
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
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
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
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)
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
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)
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
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;
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
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
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