Re: Sequence Access Method WIP
От | Petr Jelinek |
---|---|
Тема | Re: Sequence Access Method WIP |
Дата | |
Msg-id | 54B52504.6050608@2ndquadrant.com обсуждение исходный текст |
Ответ на | Re: Sequence Access Method WIP (Tomas Vondra <tomas.vondra@2ndquadrant.com>) |
Список | pgsql-hackers |
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
В списке pgsql-hackers по дате отправления: