Re: [HACKERS] sequence data type

On Tue, Jan 10, 2017 at 5:03 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> On 1/8/17 2:39 PM, Steve Singer wrote:
>> The only concern I have with the functionality is that there isn't a way
>> to change the type of a sequence.
>
> If we implement the NEXT VALUE FOR expression (or anything similar that
> returns a value from the sequence), then the return type of that
> expression would be the type of the sequence.  Then, changing the type
> of the sequence would require reparsing all expressions involving the
> sequence.  This could probably be sorted out somehow, but I don't want
> to be too lax now and cause problems for later features.  There is a
> similar case, namely changing the return type of a function, which we
> also prohibit.
>
>> @@ -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?
>
> Yes, good catch.

+           ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("MINVALUE (%s) is too large for sequence data type %s",
+                       bufm, format_type_be(seqform->seqtypid))));
"too large" for a minimum value, really? You may want to add a comment
to say that the _MAX values are used for symmetry.

+           /* We use the _MAX constants for symmetry. */
+           if (seqform->seqtypid == INT2OID)
+               seqform->seqmin = -PG_INT16_MAX;
+           else if (seqform->seqtypid == INT4OID)
+               seqform->seqmin = -PG_INT32_MAX;
+           else
+               seqform->seqmin = -PG_INT64_MAX;
Hm. Is symmetry an important properly for sequences? It seems to me
that if we map with the data types we had better use the MIN values
instead for consistency. So the concept of this patch is rather weird
and would introduce an exception with the rest of the system just for
sequences.

(There are no tests for cycles).
-- 
Michael