Re: Sequence Access Method WIP
От | Tomas Vondra |
---|---|
Тема | Re: Sequence Access Method WIP |
Дата | |
Msg-id | 54B50E85.2060901@2ndquadrant.com обсуждение исходный текст |
Ответ на | Re: Sequence Access Method WIP (Petr Jelinek <petr@2ndquadrant.com>) |
Ответы |
Re: Sequence Access Method WIP
(Petr Jelinek <petr@2ndquadrant.com>)
|
Список | pgsql-hackers |
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? (2) NITPICK: Maybe we could use access_method in the docs (instead of sequence_access_method), as the 'sequence' part isclear from the context? (3) Why index_reloptions / sequence_reloptions when both do exactly the same thing (call to common_am_reloptions)? I'dunderstand this if the patch(es) then change the sequence_reloptions() but that's not happening. Maybe that's expectedto happen? (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 fixedafter creating the sequence? (5) DOCS/NITPICK: SeqAM / SeqAm ... (probably should be the same?) (6) cache lookup failed for relation %u I believe this should reference 'sequence access method' instead of a relation. (7) seqam_init I believe this is a bug (not setting nulls[4] but twice nulls[3]): + fcinfo.argnull[0] = seqparams == NULL; + fcinfo.argnull[1] = reloptions_parsed == NULL; + fcinfo.argnull[2] = false; + fcinfo.argnull[3] = false; + fcinfo.argnull[3] = false; (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 onfaith. In which situation this happens? Wouldn't it be better to simply enforce the transaction and fail otherwise? regards -- Tomas Vondra http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
В списке pgsql-hackers по дате отправления: