On Wed, Jun 8, 2011 at 6:36 PM, Brar Piening <b...@gmx.de> wrote:
> I tried to look at everything and cover everthing but please consider that
> this is my first review so please have a second look at it!

I took a look at this as well, and I didn't encounter any problems
either. The patch is surprisingly small, but it looks like all the
documentation spots got covered, and I didn't see any missing pieces;
pg_dump copes fine, I didn't try pg_upgrade but I wouldn't expect
problems there either.

Actually, the one piece I think could be added is a regression test. I
didn't see any existing tests that covered bigserial/serial8, either,
so I went ahead and added a few tests for smallerial and bigserial. I
didn't see the testcases Brar posted at first, or I might have adopted
a few from there, but this should cover basic use of smallserial.

Josh
diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out
index 13e1565..968fcce 100644
*** a/src/test/regress/expected/sequence.out
--- b/src/test/regress/expected/sequence.out
*************** SELECT * FROM serialTest;
*** 16,21 ****
--- 16,99 ----
   force | 100
  (3 rows)
  
+ -- test smallserial / bigserial
+ CREATE TABLE serialTest2 (f1 text, f2 serial, f3 smallserial, f4 serial2,
+   f5 bigserial, f6 serial8);
+ NOTICE:  CREATE TABLE will create implicit sequence "serialtest2_f2_seq" for serial column "serialtest2.f2"
+ NOTICE:  CREATE TABLE will create implicit sequence "serialtest2_f3_seq" for serial column "serialtest2.f3"
+ NOTICE:  CREATE TABLE will create implicit sequence "serialtest2_f4_seq" for serial column "serialtest2.f4"
+ NOTICE:  CREATE TABLE will create implicit sequence "serialtest2_f5_seq" for serial column "serialtest2.f5"
+ NOTICE:  CREATE TABLE will create implicit sequence "serialtest2_f6_seq" for serial column "serialtest2.f6"
+ INSERT INTO serialTest2 (f1)
+   VALUES ('test_defaults');
+ INSERT INTO serialTest2 (f1, f2, f3, f4, f5, f6)
+   VALUES ('test_max_vals', 2147483647, 32767, 32767, 9223372036854775807,
+           9223372036854775807),
+          ('test_min_vals', -2147483648, -32768, -32768, -9223372036854775808,
+           -9223372036854775808);
+ -- All these INSERTs should fail:
+ INSERT INTO serialTest2 (f1, f3)
+   VALUES ('bogus', -32769);
+ ERROR:  smallint out of range
+ INSERT INTO serialTest2 (f1, f4)
+   VALUES ('bogus', -32769);
+ ERROR:  smallint out of range
+ INSERT INTO serialTest2 (f1, f3)
+   VALUES ('bogus', 32768);
+ ERROR:  smallint out of range
+ INSERT INTO serialTest2 (f1, f4)
+   VALUES ('bogus', 32768);
+ ERROR:  smallint out of range
+ INSERT INTO serialTest2 (f1, f5)
+   VALUES ('bogus', -9223372036854775809);
+ ERROR:  bigint out of range
+ INSERT INTO serialTest2 (f1, f6)
+   VALUES ('bogus', -9223372036854775809);
+ ERROR:  bigint out of range
+ INSERT INTO serialTest2 (f1, f5)
+   VALUES ('bogus', 9223372036854775808);
+ ERROR:  bigint out of range
+ INSERT INTO serialTest2 (f1, f6)
+   VALUES ('bogus', 9223372036854775808);
+ ERROR:  bigint out of range
+ SELECT * FROM serialTest2 ORDER BY f2 ASC;
+       f1       |     f2      |   f3   |   f4   |          f5          |          f6          
+ ---------------+-------------+--------+--------+----------------------+----------------------
+  test_min_vals | -2147483648 | -32768 | -32768 | -9223372036854775808 | -9223372036854775808
+  test_defaults |           1 |      1 |      1 |                    1 |                    1
+  test_max_vals |  2147483647 |  32767 |  32767 |  9223372036854775807 |  9223372036854775807
+ (3 rows)
+ 
+ SELECT nextval('serialTest2_f2_seq');
+  nextval 
+ ---------
+        2
+ (1 row)
+ 
+ SELECT nextval('serialTest2_f3_seq');
+  nextval 
+ ---------
+        2
+ (1 row)
+ 
+ SELECT nextval('serialTest2_f4_seq');
+  nextval 
+ ---------
+        2
+ (1 row)
+ 
+ SELECT nextval('serialTest2_f5_seq');
+  nextval 
+ ---------
+        2
+ (1 row)
+ 
+ SELECT nextval('serialTest2_f6_seq');
+  nextval 
+ ---------
+        2
+ (1 row)
+ 
  -- basic sequence operations using both text and oid references
  CREATE SEQUENCE sequence_test;
  SELECT nextval('sequence_test'::text);
*************** SELECT nextval('sequence_test2');
*** 221,231 ****
  (1 row)
  
  -- Information schema
! SELECT * FROM information_schema.sequences WHERE sequence_name IN ('sequence_test2');
!  sequence_catalog | sequence_schema | sequence_name  | data_type | numeric_precision | numeric_precision_radix | numeric_scale | start_value | minimum_value | maximum_value | increment | cycle_option 
! ------------------+-----------------+----------------+-----------+-------------------+-------------------------+---------------+-------------+---------------+---------------+-----------+--------------
!  regression       | public          | sequence_test2 | bigint    |                64 |                       2 |             0 | 32          | 5             | 36            | 4         | YES
! (1 row)
  
  -- Test comments
  COMMENT ON SEQUENCE asdf IS 'won''t work';
--- 299,317 ----
  (1 row)
  
  -- Information schema
! SELECT * FROM information_schema.sequences WHERE sequence_name IN
!   ('sequence_test2', 'serialtest2_f2_seq', 'serialtest2_f3_seq',
!    'serialtest2_f4_seq', 'serialtest2_f5_seq', 'serialtest2_f6_seq')
!   ORDER BY sequence_name ASC;
!  sequence_catalog | sequence_schema |   sequence_name    | data_type | numeric_precision | numeric_precision_radix | numeric_scale | start_value | minimum_value |    maximum_value    | increment | cycle_option 
! ------------------+-----------------+--------------------+-----------+-------------------+-------------------------+---------------+-------------+---------------+---------------------+-----------+--------------
!  regression       | public          | sequence_test2     | bigint    |                64 |                       2 |             0 | 32          | 5             | 36                  | 4         | YES
!  regression       | public          | serialtest2_f2_seq | bigint    |                64 |                       2 |             0 | 1           | 1             | 9223372036854775807 | 1         | NO
!  regression       | public          | serialtest2_f3_seq | bigint    |                64 |                       2 |             0 | 1           | 1             | 9223372036854775807 | 1         | NO
!  regression       | public          | serialtest2_f4_seq | bigint    |                64 |                       2 |             0 | 1           | 1             | 9223372036854775807 | 1         | NO
!  regression       | public          | serialtest2_f5_seq | bigint    |                64 |                       2 |             0 | 1           | 1             | 9223372036854775807 | 1         | NO
!  regression       | public          | serialtest2_f6_seq | bigint    |                64 |                       2 |             0 | 1           | 1             | 9223372036854775807 | 1         | NO
! (6 rows)
  
  -- Test comments
  COMMENT ON SEQUENCE asdf IS 'won''t work';
*************** REVOKE ALL ON seq3 FROM seq_user;
*** 289,293 ****
--- 375,391 ----
  SELECT lastval();
  ERROR:  permission denied for sequence seq3
  ROLLBACK;
+ -- Sequences should get wiped out as well:
+ DROP TABLE serialTest, serialTest2;
+ -- Make sure sequences are gone:
+ SELECT * FROM information_schema.sequences WHERE sequence_name IN
+   ('sequence_test2', 'serialtest2_f2_seq', 'serialtest2_f3_seq',
+    'serialtest2_f4_seq', 'serialtest2_f5_seq', 'serialtest2_f6_seq')
+   ORDER BY sequence_name ASC;
+  sequence_catalog | sequence_schema | sequence_name  | data_type | numeric_precision | numeric_precision_radix | numeric_scale | start_value | minimum_value | maximum_value | increment | cycle_option 
+ ------------------+-----------------+----------------+-----------+-------------------+-------------------------+---------------+-------------+---------------+---------------+-----------+--------------
+  regression       | public          | sequence_test2 | bigint    |                64 |                       2 |             0 | 32          | 5             | 36            | 4         | YES
+ (1 row)
+ 
  DROP USER seq_user;
  DROP SEQUENCE seq;
diff --git a/src/test/regress/sql/sequence.sql b/src/test/regress/sql/sequence.sql
index 29ea691..97ffb60 100644
*** a/src/test/regress/sql/sequence.sql
--- b/src/test/regress/sql/sequence.sql
*************** INSERT INTO serialTest VALUES ('wrong',
*** 11,16 ****
--- 11,62 ----
  
  SELECT * FROM serialTest;
  
+ -- test smallserial / bigserial
+ CREATE TABLE serialTest2 (f1 text, f2 serial, f3 smallserial, f4 serial2,
+   f5 bigserial, f6 serial8);
+ 
+ INSERT INTO serialTest2 (f1)
+   VALUES ('test_defaults');
+ 
+ INSERT INTO serialTest2 (f1, f2, f3, f4, f5, f6)
+   VALUES ('test_max_vals', 2147483647, 32767, 32767, 9223372036854775807,
+           9223372036854775807),
+          ('test_min_vals', -2147483648, -32768, -32768, -9223372036854775808,
+           -9223372036854775808);
+ 
+ -- All these INSERTs should fail:
+ INSERT INTO serialTest2 (f1, f3)
+   VALUES ('bogus', -32769);
+ 
+ INSERT INTO serialTest2 (f1, f4)
+   VALUES ('bogus', -32769);
+ 
+ INSERT INTO serialTest2 (f1, f3)
+   VALUES ('bogus', 32768);
+ 
+ INSERT INTO serialTest2 (f1, f4)
+   VALUES ('bogus', 32768);
+ 
+ INSERT INTO serialTest2 (f1, f5)
+   VALUES ('bogus', -9223372036854775809);
+ 
+ INSERT INTO serialTest2 (f1, f6)
+   VALUES ('bogus', -9223372036854775809);
+ 
+ INSERT INTO serialTest2 (f1, f5)
+   VALUES ('bogus', 9223372036854775808);
+ 
+ INSERT INTO serialTest2 (f1, f6)
+   VALUES ('bogus', 9223372036854775808);
+ 
+ SELECT * FROM serialTest2 ORDER BY f2 ASC;
+ 
+ SELECT nextval('serialTest2_f2_seq');
+ SELECT nextval('serialTest2_f3_seq');
+ SELECT nextval('serialTest2_f4_seq');
+ SELECT nextval('serialTest2_f5_seq');
+ SELECT nextval('serialTest2_f6_seq');
+ 
  -- basic sequence operations using both text and oid references
  CREATE SEQUENCE sequence_test;
  
*************** SELECT nextval('sequence_test2');
*** 86,92 ****
  SELECT nextval('sequence_test2');
  
  -- Information schema
! SELECT * FROM information_schema.sequences WHERE sequence_name IN ('sequence_test2');
  
  -- Test comments
  COMMENT ON SEQUENCE asdf IS 'won''t work';
--- 132,141 ----
  SELECT nextval('sequence_test2');
  
  -- Information schema
! SELECT * FROM information_schema.sequences WHERE sequence_name IN
!   ('sequence_test2', 'serialtest2_f2_seq', 'serialtest2_f3_seq',
!    'serialtest2_f4_seq', 'serialtest2_f5_seq', 'serialtest2_f6_seq')
!   ORDER BY sequence_name ASC;
  
  -- Test comments
  COMMENT ON SEQUENCE asdf IS 'won''t work';
*************** REVOKE ALL ON seq3 FROM seq_user;
*** 118,122 ****
--- 167,180 ----
  SELECT lastval();
  ROLLBACK;
  
+ -- Sequences should get wiped out as well:
+ DROP TABLE serialTest, serialTest2;
+ 
+ -- Make sure sequences are gone:
+ SELECT * FROM information_schema.sequences WHERE sequence_name IN
+   ('sequence_test2', 'serialtest2_f2_seq', 'serialtest2_f3_seq',
+    'serialtest2_f4_seq', 'serialtest2_f5_seq', 'serialtest2_f6_seq')
+   ORDER BY sequence_name ASC;
+ 
  DROP USER seq_user;
  DROP SEQUENCE seq;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to