Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Allow CREATE FOREIGN TABLE to include SERIAL columns.

  1. Add some regression tests for SEQUENCE

    Robins Tharakan <tharakan@gmail.com> — 2013-03-12T16:35:05Z

    Hi,

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.
--
Robins
Tharakan

  2. Re: Add some regression tests for SEQUENCE

    Albe Laurenz <laurenz.albe@wien.gv.at> — 2013-03-13T08:19:43Z

    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

  3. Re: Add some regression tests for SEQUENCE

    Robins Tharakan <tharakan@gmail.com> — 2013-03-13T10:11:53Z

    Thanks Laurenz.

Would correct these (and readup) before submitting next patch.

--
Robins
Tharakan


On 13 March 2013 13:49, Albe Laurenz <laurenz.albe@wien.gv.at> wrote:

> 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
>

  4. Re: Add some regression tests for SEQUENCE

    Robins Tharakan <tharakan@gmail.com> — 2013-03-15T20:33:37Z

    Hi,

I've added some regression tests for SEQUENCE. A cumulative patch is
attached.

Barring a (still to decipher) function seq_redo() and trying to learn how
to actually test it, this takes care of most branches of (
src/backend/commands/sequence.c) taking code-coverage (of 'make check') to
~95%.

Any feedback is more than welcome.
--
Robins
Tharakan


On 13 March 2013 15:41, robins <tharakan@gmail.com> wrote:

> Thanks Laurenz.
>
> Would correct these (and readup) before submitting next patch.
>
> --
> Robins
> Tharakan
>
>
> On 13 March 2013 13:49, Albe Laurenz <laurenz.albe@wien.gv.at> wrote:
>
>> 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
>>
>
>

  5. Re: Add some regression tests for SEQUENCE

    Robins Tharakan <tharakan@gmail.com> — 2013-03-18T22:10:21Z

    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%.

--
Robins Tharakan


On 16 March 2013 02:03, robins <tharakan@gmail.com> wrote:

> Hi,
>
> I've added some regression tests for SEQUENCE. A cumulative patch is
> attached.
>
> Barring a (still to decipher) function seq_redo() and trying to learn how
> to actually test it, this takes care of most branches of (
> src/backend/commands/sequence.c) taking code-coverage (of 'make check')
> to ~95%.
>
> Any feedback is more than welcome.
>

  6. Re: Add some regression tests for SEQUENCE

    Josh Kupershmidt <schmiddy@gmail.com> — 2013-03-18T22:37:52Z

    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

  7. Re: Add some regression tests for SEQUENCE

    Robins Tharakan <tharakan@gmail.com> — 2013-03-18T23:07:25Z

    Duh. Apologies. That's what happens when you make that 1 last change.

Please find an updated patch.

--
Robins Tharakan


On 19 March 2013 04:07, Josh Kupershmidt <schmiddy@gmail.com> wrote:

> 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
>

  8. Re: Add some regression tests for SEQUENCE

    Robins Tharakan <tharakan@gmail.com> — 2013-05-07T22:40:11Z

    Hi,

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

--
Robins Tharakan

  9. Re: Add some regression tests for SEQUENCE

    Fabien COELHO <coelho@cri.ensmp.fr> — 2013-05-08T07:55:07Z

    > 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.

  10. Re: Add some regression tests for SEQUENCE

    Josh Berkus <josh@agliodbs.com> — 2013-06-28T20:53:46Z

    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

  11. Re: Add some regression tests for SEQUENCE

    Robins Tharakan <tharakan@gmail.com> — 2013-06-28T21:15:30Z

    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.

--
Robins Tharakan


On 28 June 2013 15:53, Josh Berkus <josh@agliodbs.com> wrote:

> 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
>

  12. Re: Add some regression tests for SEQUENCE

    Josh Berkus <josh@agliodbs.com> — 2013-06-28T21:22:15Z

    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

  13. Re: Add some regression tests for SEQUENCE

    Josh Berkus <josh@agliodbs.com> — 2013-06-28T21:28:49Z

    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

  14. Re: Add some regression tests for SEQUENCE

    Robert Haas <robertmhaas@gmail.com> — 2013-07-03T15:13:00Z

    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

  15. Re: Add some regression tests for SEQUENCE

    Robins Tharakan <tharakan@gmail.com> — 2013-07-07T13:01:51Z

    On 3 July 2013 10:13, Robert Haas <robertmhaas@gmail.com> wrote:

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

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

--
Robins Tharakan

  16. Re: Add some regression tests for SEQUENCE

    Fabien COELHO <coelho@cri.ensmp.fr> — 2013-07-09T15:41:56Z

    > 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.

  17. Re: Add some regression tests for SEQUENCE

    Robins Tharakan <tharakan@gmail.com> — 2013-07-15T13:09:08Z

    On 9 July 2013 08:41, Fabien COELHO <coelho@cri.ensmp.fr> wrote:

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

Thanks Fabien. This was a wrong attachment to the email.
Please find attached the updated patch (I've renamed v5 as v6 for clarity).

git reset --hard HEAD
git pull
patch -p1 < ../regress_sequence_v6.patch
patch -p1 -R < ../regress_sequence_v6.patch
patch -p1 < ../regress_sequence_v6.patch
make clean
./configure --enable-depend --enable-coverage --enable-cassert
--enable-debug
make -j3 check


--
Robins Tharakan

  18. Re: Add some regression tests for SEQUENCE

    Fabien COELHO <coelho@cri.ensmp.fr> — 2013-07-18T11:46:11Z

    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 CACHE 1;
   SELECT NEXTVAL('seq31');
   DROP SEQUENCE seq31;

-- 
Fabien.