Thread: Add some regression tests for SEQUENCE

robins wrote:
> Attached is a small patch to test corner cases related to Sequences (basically aimed at increasing
> code-coverage of sequence.sql in regression tests).
>
> Look forward to any and all feedback.

Looks ok except that the patch is backwards
(all added lines start with "-").  I found a typo:
"exit" instead of "exist".

You should add the patch to the next commitfest
(http://wiki.postgresql.org/wiki/Submitting_a_Patch).

Yours,
Laurenz Albe

On Mon, Mar 18, 2013 at 3:10 PM, Robins Tharakan <tharakan@gmail.com> wrote:
> Hi,
>
> Please find an updated patch (reworked on the names of SEQUENCES / ROLES /
> SCHEMA etc.)
> Takes code-coverage of 'make check' for SEQUENCE to ~95%.

There is a typo difference between sequence.out and sequence.sql
causing the test to fail:

+-- Should fail since seq5 shouldn't exist
...
+-- Should fail since seq5 shouldn't exit

Josh

> Have provided an updated patch as per Fabien's recent response on 
> Commitfest site. Any and all feedback is appreciated.

Review:

This patch works for me.

It adds valuable sequence test cases, especially trying corner cases with 
expected errors and permission denials.

I suggest to accept it.

-- 
Fabien.

On 05/07/2013 03:40 PM, Robins Tharakan wrote:
> Hi,
> 
> Have provided an updated patch as per Fabien's recent response on
> Commitfest site.
> Any and all feedback is appreciated.

The updated patch is giving a FAILURE for me:

parallel group (19 tests):  limit temp plancache conversion rowtypes
prepare without_oid copy2 xml returning rangefuncs polymorphism with
domain truncate largeobject sequence alter_table plpgsql    plancache                ... ok    limit
...ok    plpgsql                  ... ok    copy2                    ... ok    temp                     ... ok
domain                  ... ok    rangefuncs               ... ok    prepare                  ... ok    without_oid
        ... ok    conversion               ... ok    truncate                 ... ok    alter_table              ... ok
  sequence                 ... FAILED
 

Thoughts?

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On 06/28/2013 02:15 PM, Robins Tharakan wrote:
> Seems like thats because of a recent (15th May 2013) patch
> (b14206862278347a379f2bb72d92d16fb9dcea45) that changed the error message
> that is printed. Its just one line of difference actually.
> 
> Let me know if this is otherwise good to go.
> I'll checkout the latest revision and submit this patch again.
> 

I was only checking test timing, per my earlier email.  I haven't looked
at the tests themselves at all.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On 06/28/2013 02:15 PM, Robins Tharakan wrote:
> Seems like thats because of a recent (15th May 2013) patch
> (b14206862278347a379f2bb72d92d16fb9dcea45) that changed the error message
> that is printed. Its just one line of difference actually.
> 
> Let me know if this is otherwise good to go.
> I'll checkout the latest revision and submit this patch again.
> 

I was only checking test timing, per my earlier email.  I haven't looked
at the tests themselves at all.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Tue, May 7, 2013 at 6:40 PM, Robins Tharakan <tharakan@gmail.com> wrote:
> Have provided an updated patch as per Fabien's recent response on Commitfest
> site.
> Any and all feedback is appreciated.

I think you should rename the roles used here to regress_rol_seq1 etc.
to match the CREATE OPERATOR patch.

And you need to update the expected output.

Setting this one to "Waiting on Author".

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

> Please find updated patch:
> - 'make check' successful with recent changes
> - Renamed ROLEs as per feedback

Sorry, I took the wrong thread.

I do not see any difference between both "regress_sequence_v[45].patch".
I guess you sent the earlier version.

-- 
Fabien.

Hello Robins,

> Thanks Fabien. This was a wrong attachment to the email.

This patch works for me (applied, tested).

However, some remarks:

seq4: should it check something? How do you know that OWNED BY did 
anything?

regress_role_seq2: shoult check that the sequence owner is the table 
owner?

seq12/seq14: is it twice the same tests??

seq13/seq15: idem??

I still do not know what "asdf" means... it is about the qwerty keyboard?
What about something explicit, like regress_seq_undefined?

seq22: remove the "syntax error" check at the end, pg people do not want 
syntax error checks.


Also, here is a proposal for testing that CACHE is working:
  -- check CACHE operation by resetting the sequence cache size  CREATE SEQUENCE seq31 CACHE 10;  -- 1 to 10 are
preallocated SELECT NEXTVAL('seq31');  -- reset cache, 2..10 are lost, should start again from 11  ALTER SEQUENCE seq31
CACHE1;  SELECT NEXTVAL('seq31');  DROP SEQUENCE seq31;
 

-- 
Fabien.