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

Le 03/11/2010 06:31, Dave Page a écrit :
> On Tue, Nov 2, 2010 at 11:38 AM, Guillaume Lelarge
> <guillaume(at)lelarge(dot)info> wrote:
>> Le 02/11/2010 07:18, Guillaume Lelarge a écrit :
>>> Le 02/11/2010 05:49, Dave Page a écrit :
>>>> On Mon, Nov 1, 2010 at 2:51 PM, Guillaume Lelarge
>>>> <guillaume(at)lelarge(dot)info> wrote:
>>>>> Le 31/10/2010 09:44, Guillaume Lelarge a écrit :
>>>>>> Le 31/10/2010 00:39, Dave Page a écrit :
>>>>>>> On Sun, Oct 31, 2010 at 1:56 AM, Guillaume Lelarge
>>>>>>> <guillaume(at)lelarge(dot)info> wrote:
>>>>>>>> Le 30/10/2010 10:25, Dave Page a écrit :
>>>>>>>>
>>>>>>>>> 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".
>>>>>>>
>>>>>>> That's knowledge I'd rather avoid hardwiring into the lower level
>>>>>>> machinery here.
>>>>>>>
>>>>>>
>>>>>> So do I. I tried a few things yesterday. Changing the apply() and
>>>>>> GetSql() parameters imply to change all GetSql for all dlg* source code.
>>>>>> That will be quite an invasive patch.
>>>>>>
>>>>>
>>>>> I've done the "split-the-queries" function. Seems to work great, but
>>>>> still doesn't cover dollar quoting. Anyway, it's less ugly than I
>>>>> thought. The interesting part is dlgProperty::SplitQueries(). Would love
>>>>> to get comments :)
>>>>
>>>> I just eyeballed the patch - if I'm reading it right, it splits *all*
>>>> queries and executes each part individually. Is that correct?
>>>>
>>>
>>> Right.
>>>
>>>> If so, we're going to need that flag. Most of the time, we want all
>>>> the query parts to be executed atomically, otherwise if we get and
>>>> error (particularly when using the Apply button where there is one),
>>>> the dialogue won't know what parts of the update work and what didn't,
>>>> and thus will have a difficult job refreshing the display
>>>> appropriately.
>>>>
>>>
>>> Yeah, that was the part I wanted to work on yesterday. I have an idea on
>>> this, I actually have the code, but it doesn't work :-/ Need some more work.
>>>
>>
>> Done. I'm actually quite happy with how it turns out. Less nasty then I
>> previously thought. Complete patch attached (alterenum_v1.patch). The
>> last part of the work is available on the patch 0001* attached to this
>> email. Or you can get a look at my branch on github
>> (http://github.com/gleu/pgadmin3/commits/ticket269).
>>
>> Probably I should change the method name... WannaSplitQueries is
>> probably not the best one we could find :)
>
> Seems like a nice solution. And the name is descriptive, if nothing else :-)
>

Commited. I you came with another name, it'll be a simple change.

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