From: | Petr Jelinek <petr(at)2ndquadrant(dot)com> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Sequence Access Method WIP |
Date: | 2015-01-13 14:00:36 |
Message-ID: | 54B52504.6050608@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
From | Date | Subject | |
---|---|---|---|
Next Message | David Fetter | 2015-01-13 14:00:47 | Re: POLA violation with \c service= |
Previous Message | Stephen Frost | 2015-01-13 13:50:19 | Re: WITH CHECK and Column-Level Privileges |