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.
Thanks!
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
context?
Yes.
(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:
http://www.postgresql.org/mailpref/pgsql-hackers
