On 11/8/16 6:43 PM, Andreas Karlsson wrote:
> - A worry is that it might get a bit confusing to have both the future
> catalog pg_sequence and the view pg_sequences.
We already have this in other cases: pg_index/pg_indexes,
pg_user_mapping/pg_user_mappings. It's an established naming system by now.
> - I think it would be useful to include is_cycled in the view.
It's there under the name "cycle".
> - When creating a temporary sequences and then running "SELECT * FROM
> pg_sequences" in another session I get the following error.
>
> ERROR: cannot access temporary tables of other sessions
Fixed that by adding pg_is_other_temp_schema() to the view definition.
We use that in the information schema but not in the system views so
far. That might be worth looking into.
> - Shouldn't last_value be NULL directly after we have created the
> sequence but nobody has called nextval() yet?
>
> - I noticed that last_value includes the cached values, but that also
> seems to me like the correct thing to do.
The documentation now emphasizes that this is the value stored on disk.
This matches what Oracle does.
> - I do not like the name of the new function, lastval(regclass). I think
> like you suggested it would be better with something more verbose.
> sequence_lastval()? sequence_last_value()?
changed
> - There is an XXX comment still in the code. It is about the name of the
> lastval1() function.
fixed
> - The documentation does not mention the last_value column.
fixed
> - The extra empty line after "</table>" does not fit with the formatting
> of the rest of the SGML file.
fixed
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services