Re: [HACKERS] sequence data type

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