Re: Ticket 269: Add support for 9.1 ALTER TYPE new syntax for enum

Le 30/10/2010 10:25, Dave Page a écrit :
> On Sat, Oct 30, 2010 at 4:51 PM, Guillaume Lelarge
> <guillaume(at)lelarge(dot)info> wrote:
>> Le 30/10/2010 08:18, Dave Page a écrit :
>>> On Sat, Oct 30, 2010 at 4:07 PM, Guillaume Lelarge
>>> <guillaume(at)lelarge(dot)info> wrote:
>>>> Le 29/10/2010 21:56, Guillaume Lelarge a écrit :
>>>>> Le 29/10/2010 21:11, Guillaume Lelarge a écrit :
>>>>>> [...]
>>>>>> This patch adds support to the new ALTER TYPE syntax in 9.1 for enums.
>>>>>>
>>>>>> It's working great except one thing. If a user wants to add two labels,
>>>>>> we're screwed because we can't do two "ALTER TYPE ... ADD" statements in
>>>>>> the same query execution. Any idea how to solve this? the only way I
>>>>>> found would be to disallow adding two labels at once but it results on a
>>>>>> less interesting feature.
>>>>>>
>>>>>
>>>>> So I was wrong. The issue is that we can't issue this statement in a
>>>>> explicit transaction. Any idea how to solve the "don't send begin/end
>>>>> statements"?
>>>>>
>>>>
>>>> The only idea I have is to make dlgType a two-SQL-boxes dialog and
>>>> modify the dlgProperty::apply() method so that if there is "ALTER TYPE
>>>> ... ADD {BEFORE | AFTER}" statements, they get splitted and fired
>>>> individualy. I didn't yet write the code to split the statement, but it
>>>> will surely be ugly.
>>>>
>>>> Any objection on doing this? or better idea to fix this issue?
>>>
>>> I don't understand why we can't do all this at once - what's the
>>> problem exactly? Normally we only split statements when they're not
>>> atomic and cannot be rolled back.
>>>
>>
>> Here is the error message I got:
>>
>> ERROR: ALTER TYPE ... ADD cannot be executed from a function or
>> multi-command string
>
> I just checked the docs, and wow - that seems like a major step
> backwards compared to our normal strive for transactional DDL.
>

Yeah. I wonder why they did it this way, but had no time yet to check
that on -hackers.

>> So, the issue we have is that we can't execute two of them at the same
>> time. We need to split the ALTER TYPE statements to make sure we execute
>> them one by one. We could have used the two-sql-textboxes without
>> actually splitting the query but in this case, a user will only be able
>> to add two new labels to an enum datatype. What happens if he wants to
>> add three of them?
>
> Yeah, that's really nasty. I guess we need split the commands at ;.

Yeah. If it's not between quotes. I don't like it at all, but I don't
see another way of doing it.

> I guess we should pass a flag down somehow to tell the function that
> executes the query to do that and then we could also potentially get
> rid of the double SQL boxes. I'm not looking at the code, but I
> suspect that'll be nasty.
>

We actually aren't required to add such a flag. We can check if the
query contains "ALTER TYPE", and "ADD AFTER" or "ADD BEFORE".

But the flag would be a good idea if we need to split the statements for
other kind of queries (we don't need now, but how can I be sure it won't
happen in a new patch?).

I'm going to work on this before having dinner.

>> So I really think we need to split the ALTER TYPE statements. We
>> probably can borrow the code from psql for this.
>
> Well psql has a full blown parser doesn't it? I'm not sure we want to
> go that far if we can help it.
>

Yes, it has. I checked after sending the mail. And I don't want to add
that code to pgAdmin :)

--
Guillaume
http://www.postgresql.fr
http://dalibo.com