Re: [HACKERS] [COMMITTERS] pgsql: Add DISCARD SEQUENCES command.

2013-10-07 Thread Michael Paquier 
On Tue, Oct 8, 2013 at 4:57 AM, Robert Haas  wrote:
> On Sat, Oct 5, 2013 at 8:10 AM, Michael Paquier
>  wrote:
>> Here is the test case failing:
>> =# create sequence foo;
>> CREATE SEQUENCE
>> =# select nextval('foo');
>>  nextval
>> -
>>1
>> (1 row)
>> =# discard sequences ;
>> DISCARD SEQUENCES
>> =# select currval('foo');
>> ERROR:  55000: currval of sequence "foo" is not yet defined in this session
>> LOCATION:  currval_oid, sequence.c:780
>> =# select lastval();
>> The connection to the server was lost. Attempting reset: Failed.
>
> Thanks.  I have pushed a fix that I hope will be sufficient.
It is fine now. Thanks for the fix.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: Add DISCARD SEQUENCES command.

2013-10-07 Thread Robert Haas 
On Sat, Oct 5, 2013 at 8:10 AM, Michael Paquier
 wrote:
> Here is the test case failing:
> =# create sequence foo;
> CREATE SEQUENCE
> =# select nextval('foo');
>  nextval
> -
>1
> (1 row)
> =# discard sequences ;
> DISCARD SEQUENCES
> =# select currval('foo');
> ERROR:  55000: currval of sequence "foo" is not yet defined in this session
> LOCATION:  currval_oid, sequence.c:780
> =# select lastval();
> The connection to the server was lost. Attempting reset: Failed.

Thanks.  I have pushed a fix that I hope will be sufficient.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: Add DISCARD SEQUENCES command.

2013-10-06 Thread Michael Paquier 
On Sun, Oct 6, 2013 at 1:25 PM, Alvaro Herrera  wrote:
> Michael Paquier escribió:
>
>> =# discard sequences ;
>> DISCARD SEQUENCES
>> =# select currval('foo');
>> ERROR:  55000: currval of sequence "foo" is not yet defined in this session
>> LOCATION:  currval_oid, sequence.c:780
>> =# select lastval();
>> The connection to the server was lost. Attempting reset: Failed.
>
> I wonder if it would be better to have this "seqtab" thing be a dlist,
> and move the most recently used item to head position on nextval().
> That way we don't have to maintain last_used_seq separately.
Indeed, this could simplify a bit the code.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: Add DISCARD SEQUENCES command.

2013-10-05 Thread Alvaro Herrera 
Michael Paquier escribió:

> =# discard sequences ;
> DISCARD SEQUENCES
> =# select currval('foo');
> ERROR:  55000: currval of sequence "foo" is not yet defined in this session
> LOCATION:  currval_oid, sequence.c:780
> =# select lastval();
> The connection to the server was lost. Attempting reset: Failed.

I wonder if it would be better to have this "seqtab" thing be a dlist,
and move the most recently used item to head position on nextval().
That way we don't have to maintain last_used_seq separately.

I also just noticed that the DISCARD SEQUENCE patch made a "NOTE:"
comment in init_sequence be outdated.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: Add DISCARD SEQUENCES command.

2013-10-05 Thread Michael Paquier 
Hi,

While having a look at this feature I found an assertion failure
(commit 1310d4c):
#2  0x00010eb7856d in ExceptionalCondition
(conditionName=0x10ec3790b "!(last_used_seq->last_valid)",
errorType=0x10ebe25e0 "FailedAssertion", fileName=0x10ec37168
"sequence.c", lineNumber=809) at assert.c:54
#3  0x00010e7cfc75 in lastval (fcinfo=0x7fd5f1084900) at sequence.c:809
#4  0x00010e82d70e in ExecMakeFunctionResult
(fcache=0x7fd5f1084890, econtext=0x7fd5f1084668, isNull=0x7fd5f1085240
"~h?\f??", isDone=0x7fd5f1085380) at execQual.c:1938
#5  0x00010e82e56f in ExecEvalFunc (fcache=0x7fd5f1084890,
econtext=0x7fd5f1084668, isNull=0x7fd5f1085240 "~h?\f??",
isDone=0x7fd5f1085380) at execQual.c:2377
#6  0x00010e836d92 in ExecTargetList (targetlist=0x7fd5f1085348,
econtext=0x7fd5f1084668, values=0x7fd5f1085220, isnull=0x7fd5f1085240
"~h?\f??", itemIsDone=0x7fd5f1085380, isDone=0x7fff51659954) at
execQual.c:5244
#7  0x00010e8374ad in ExecProject (projInfo=0x7fd5f1085260,
isDone=0x7fff51659954) at execQual.c:5459
#8  0x00010e858151 in ExecResult (node=0x7fd5f1084550) at nodeResult.c:155
#9  0x00010e828bf3 in ExecProcNode (node=0x7fd5f1084550) at
execProcnode.c:373
#10 0x00010e8262fe in ExecutePlan (estate=0x7fd5f1084438,
planstate=0x7fd5f1084550, operation=CMD_SELECT, sendTuples=1 '\001',
numberTuples=0, direction=ForwardScanDirection, dest=0x7fd5f1035470)
at execMain.c:1472
#11 0x00010e8238fb in standard_ExecutorRun
(queryDesc=0x7fd5f10cfe38, direction=ForwardScanDirection, count=0) at
execMain.c:307
#12 0x00010e8236e8 in ExecutorRun (queryDesc=0x7fd5f10cfe38,
direction=ForwardScanDirection, count=0) at execMain.c:255
#13 0x00010e9dd21e in PortalRunSelect (portal=0x7fd5f10a6c38,
forward=1 '\001', count=0, dest=0x7fd5f1035470) at pquery.c:946
#14 0x00010e9dcd96 in PortalRun (portal=0x7fd5f10a6c38,
count=9223372036854775807, isTopLevel=1 '\001', dest=0x7fd5f1035470,
altdest=0x7fd5f1035470, completionTag=0x7fff51659e00 "") at
pquery.c:790
#15 0x00010e9d4764 in exec_simple_query
(query_string=0x7fd5f1033a38 "select lastval();") at postgres.c:1048
#16 0x00010e9da617 in PostgresMain (argc=1, argv=0x7fd5f101bfc0,
dbname=0x7fd5f101be28 "ioltas", username=0x7fd5f101be08 "ioltas") at
postgres.c:3992
#17 0x00010e953756 in BackendRun (port=0x7fd5f0c06280) at postmaster.c:4083
#18 0x00010e952aec in BackendStartup (port=0x7fd5f0c06280) at
postmaster.c:3772
#19 0x00010e94dfd3 in ServerLoop () at postmaster.c:1583
#20 0x00010e94d14e in PostmasterMain (argc=3, argv=0x7fd5f0c04150)
at postmaster.c:1239
#21 0x00010e8854db in main (argc=3, argv=0x7fd5f0c04150) at main.c:196

Here is the test case failing:
=# create sequence foo;
CREATE SEQUENCE
=# select nextval('foo');
 nextval
-
   1
(1 row)
=# discard sequences ;
DISCARD SEQUENCES
=# select currval('foo');
ERROR:  55000: currval of sequence "foo" is not yet defined in this session
LOCATION:  currval_oid, sequence.c:780
=# select lastval();
The connection to the server was lost. Attempting reset: Failed.

Also, I would have expected that the value cached for lastval is also
discarded after invocating DISCARD SEQUENCES. Is the behavior below
expected?
=# create sequence foo;
CREATE SEQUENCE
=# select nextval('foo');
 nextval
-
   1
(1 row)
=# discard sequences ;
DISCARD SEQUENCES
=# select lastval();
 lastval
-
   1
(1 row)

I didn't have a look at the code, but perhaps the assertion failure I
am seeing is because of lastval not discarded as well.

Regards,
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: Add DISCARD SEQUENCES command.

2013-10-03 Thread Robert Haas 
On Thu, Oct 3, 2013 at 6:38 PM, Kevin Hale Boyes  wrote:
> My C is very rusty but the traversal of SeqTableData doesn't seem correct.
> It saves the seqtab->next pointer into next, frees seqtab and then
> dereferences it.
> Shouldn't that last line be: seqtab = next?
>
> Kevin.
>
> +/*
> + * Flush cached sequence information.
> + */
> +void
> +ResetSequenceCaches(void)
> +{
> +   SeqTableData *next;
> +
> +   while (seqtab != NULL)
> +   {
> +   next = seqtab->next;
> +   free(seqtab);
> +   seqtab = seqtab->next;
> +   }

Oops, good catch.  Will fix, thanks.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: Add DISCARD SEQUENCES command.

2013-10-03 Thread Kevin Hale Boyes 
My C is very rusty but the traversal of SeqTableData doesn't seem correct.
It saves the seqtab->next pointer into next, frees seqtab and then
dereferences it.
Shouldn't that last line be: seqtab = next?

Kevin.

+/*
+ * Flush cached sequence information.
+ */
+void
+ResetSequenceCaches(void)
+{
+   SeqTableData *next;
+
+   while (seqtab != NULL)
+   {
+   next = seqtab->next;
+   free(seqtab);
+   seqtab = seqtab->next;
+   }



On 3 October 2013 14:23, Robert Haas  wrote:

> Add DISCARD SEQUENCES command.
>
> DISCARD ALL will now discard cached sequence information, as well.
>
> Fabrízio de Royes Mello, reviewed by Zoltán Böszörményi, with some
> further tweaks by me.
>
> Branch
> --
> master
>
> Details
> ---
>
> http://git.postgresql.org/pg/commitdiff/d90ced8bb22194cbb45f58beb0961251103aeff5
>
> Modified Files
> --
> doc/src/sgml/ref/discard.sgml  |   12 +++-
> src/backend/commands/discard.c |8 +++-
> src/backend/commands/sequence.c|   16 
> src/backend/parser/gram.y  |9 -
> src/backend/tcop/utility.c |3 +++
> src/bin/psql/tab-complete.c|2 +-
> src/include/commands/sequence.h|1 +
> src/include/nodes/parsenodes.h |1 +
> src/test/regress/expected/sequence.out |3 +++
> src/test/regress/sql/sequence.sql  |2 ++
> 10 files changed, 53 insertions(+), 4 deletions(-)
>
>
> --
> Sent via pgsql-committers mailing list (pgsql-committ...@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-committers
>