On 13/01/15 13:24, Tomas Vondra wrote:
On 12.1.2015 22:33, Petr Jelinek wrote:
On 15/12/14 11:36, Petr Jelinek wrote:
On 10/12/14 03:33, Petr Jelinek wrote:
On 24/11/14 12:16, Heikki Linnakangas wrote:

About the rough edges:
- The AlterSequence is not prettiest code around as we now have to
create new relation when sequence AM is changed and I don't know how to
do that nicely
- I am not sure if I did the locking, command order and dependency
handling in AlterSequence correcly

This version does AlterSequence differently and better. Didn't attach
the gapless sequence again as that one is unchanged.

And another version, separated into patch-set of 3 patches.
First patch contains the seqam patch itself, not many changes there,
mainly docs/comments related. What I wrote in the previous email for
version 3 still applies.

I did a review of the first part today - mostly by reading through the
diff. I plan to do a bit more thorough testing in a day or two. I'll
also look at the two (much smaller) patches.


comments/questions/general nitpicking:

(1) Why treating empty string as equal to 'local'? Isn't enforcing the
     actual value a better approach?

Álvaro mentioned on IM also, I already changed it to saner normal GUC with 'local' as default value in my working copy

(2) NITPICK: Maybe we could use access_method in the docs (instead of
     sequence_access_method), as the 'sequence' part is clear from the


(3) Why index_reloptions / sequence_reloptions when both do exactly the
     same thing (call to common_am_reloptions)? I'd understand this if
     the patch(es) then change the sequence_reloptions() but that's not
     happening. Maybe that's expected to happen?

That's leftover from the original design where AM was supposed to call this, since this is not exposed to AM anymore I think it can be single function now.

(4) DOCS: Each sequence can only use one access method at a time.

     Does that mean a sequence can change the access method during it's
     lifetime? My understanding is that the AM is fixed after creating
     the sequence?

Oh, I forgot to add ALTER SEQUENCE USING into docs, you can change AM even though you probably don't want to do it often, but for migrations it's useful.

(8) check_default_seqam without a transaction

  * If we aren't inside a transaction, we cannot do database access so
  * cannot verify the name.     Must accept the value on faith.

     In which situation this happens? Wouldn't it be better to simply
     enforce the transaction and fail otherwise?

Reading postgresql.conf during startup, I don't think we want to fail in that scenario ;)

 Petr Jelinek                  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to