On 12/31/2016 01:27 AM, Peter Eisentraut wrote:
Another updated patch, with quite a bit of rebasing and some minor code polishing.


Patch applies cleanly and the tests pass
The feature seems to work as expected.

I've tried this out and it behaves as expected and desired. I also tested the PG dump changes when dumping from both 8.3 and 9.5 and tables with serial types and standalone sequences restore as I would expect.


The only concern I have with the functionality is that there isn't a way to change the type of a sequence.

For example

create table foo(id serial4);
--hmm I"m running out of id space
alter table foo alter column id type int8;
alter sequence foo_id_seq maxvalue
9223372036854775807;

2017-01-08 14:29:27.073 EST [4935] STATEMENT: alter sequence foo_id_seq maxvalue
    9223372036854775807;
ERROR: MAXVALUE (9223372036854775807) is too large for sequence data type integer

Since we allow for min/maxvalue to be changed I feel we should also allow the type to be changed.




@@ -1236,7 +1239,15 @@ init_params(ParseState *pstate, List *options, bool isInit,
        {
                DefElem    *defel = (DefElem *) lfirst(option);

-               if (strcmp(defel->defname, "increment") == 0)
+               if (strcmp(defel->defname, "as") == 0)
+               {
+                       if (as_type)
+                               ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("conflicting or redundant options")));
+                       as_type = defel;
+               }
+               else if (strcmp(defel->defname, "increment") == 0)

Should we including parser_errposition(pstate, defel->location))); like for the other errors below this?


Other than that the patch looks good



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