Обсуждение: Beta 2 Wednesday?
Any objections? Regards, Dave
Dave Page a écrit : > Any objections? > Yes, one. Sorry :) I've discussed a bit with Pavel about his problem with functions. It seems pgAdmin lacks two new parameters for CREATE FUNCTION dialog : COST and ROWS. I was on the way to add these on dlgFunction. In fact, there are two questions. First, do we want them in pgAdmin 1.8.0 ? because it means adding new strings, which is not a really great news for translators. Next, if we want them, do we wait for this patch to release beta 2 ? Regards. -- Guillaume. <!-- http://abs.traduc.org/ http://lfs.traduc.org/ http://docs.postgresqlfr.org/ -->
Guillaume Lelarge wrote: > Dave Page a écrit : >> Any objections? >> > > Yes, one. Sorry :) > > I've discussed a bit with Pavel about his problem with functions. It > seems pgAdmin lacks two new parameters for CREATE FUNCTION dialog : COST > and ROWS. No it doesn't: http://svn.pgadmin.org/cgi-bin/viewcvs.cgi?rev=6159&view=rev Regards, Dave.
Dave Page a écrit : > Guillaume Lelarge wrote: >> Dave Page a écrit : >>> Any objections? >>> >> >> Yes, one. Sorry :) >> >> I've discussed a bit with Pavel about his problem with functions. It >> seems pgAdmin lacks two new parameters for CREATE FUNCTION dialog : COST >> and ROWS. > > No it doesn't: > > http://svn.pgadmin.org/cgi-bin/viewcvs.cgi?rev=6159&view=rev > Hmmm... it seems you're right and I'm stupid :) I wonder how I could have not seen it. Thanks. PS : so, I've no objection for beta 2 on wednesday. -- Guillaume. <!-- http://abs.traduc.org/ http://lfs.traduc.org/ http://docs.postgresqlfr.org/ -->
Guillaume Lelarge a écrit :
> PS : so, I've no objection for beta 2 on wednesday.
>
In fact, I have one. I just fixed a bug on the creation of a function
without cost parameter but there's still one unfixed. It's in
dlgFunction.cpp source file, method CheckChange. I think the problem is
on these lines :
if (!isProcedure || !connection->EdbMinimumVersion(8, 0))
CheckValid(enable, cbLanguage->GetCurrentSelection() >= 0,
_("Please select language."));
There's something wrong in this test with a vanilla PostgreSQL. When I
comment these lines, I can apply my changes on a function's definition.
Otherwise, the OK button remains disabled whatever changes I could have
done. But I have absolutely no idea what's going wrong here. Dave, can
you take a look at this before building beta 2 ? Thanks.
Regards.
--
Guillaume.
<!-- http://abs.traduc.org/
http://lfs.traduc.org/
http://docs.postgresqlfr.org/ -->
Guillaume Lelarge wrote: > Dave Page a écrit : >> Guillaume Lelarge wrote: >>> Dave Page a écrit : >>>> Any objections? >>>> >>> Yes, one. Sorry :) >>> >>> I've discussed a bit with Pavel about his problem with functions. It >>> seems pgAdmin lacks two new parameters for CREATE FUNCTION dialog : COST >>> and ROWS. >> No it doesn't: >> >> http://svn.pgadmin.org/cgi-bin/viewcvs.cgi?rev=6159&view=rev >> > > Hmmm... it seems you're right and I'm stupid :) I wonder how I could > have not seen it. Stupid is a bit harsh - sleepy perhaps? Or drunk? :-) /D
Guillaume Lelarge wrote:
> Guillaume Lelarge a écrit :
>> PS : so, I've no objection for beta 2 on wednesday.
>>
>
> In fact, I have one. I just fixed a bug on the creation of a function
> without cost parameter but there's still one unfixed. It's in
> dlgFunction.cpp source file, method CheckChange. I think the problem is
> on these lines :
>
> if (!isProcedure || !connection->EdbMinimumVersion(8, 0))
> CheckValid(enable, cbLanguage->GetCurrentSelection() >= 0,
> _("Please select language."));
>
>
> There's something wrong in this test with a vanilla PostgreSQL. When I
> comment these lines, I can apply my changes on a function's definition.
> Otherwise, the OK button remains disabled whatever changes I could have
> done. But I have absolutely no idea what's going wrong here. Dave, can
> you take a look at this before building beta 2 ? Thanks.
I couldn't reproduce the problem myself (against PG 8.2.4), but I agree
that the logic in the check is in need of attention. I've committed a
slightly more sane version - please check it's OK for you.
Thanks, Dave
Dave Page a écrit :
> Guillaume Lelarge wrote:
>> Guillaume Lelarge a écrit :
>>> PS : so, I've no objection for beta 2 on wednesday.
>>>
>> In fact, I have one. I just fixed a bug on the creation of a function
>> without cost parameter but there's still one unfixed. It's in
>> dlgFunction.cpp source file, method CheckChange. I think the problem is
>> on these lines :
>>
>> if (!isProcedure || !connection->EdbMinimumVersion(8, 0))
>> CheckValid(enable, cbLanguage->GetCurrentSelection() >= 0,
>> _("Please select language."));
>>
>>
>> There's something wrong in this test with a vanilla PostgreSQL. When I
>> comment these lines, I can apply my changes on a function's definition.
>> Otherwise, the OK button remains disabled whatever changes I could have
>> done. But I have absolutely no idea what's going wrong here. Dave, can
>> you take a look at this before building beta 2 ? Thanks.
>
> I couldn't reproduce the problem myself (against PG 8.2.4), but I agree
> that the logic in the check is in need of attention. I've committed a
> slightly more sane version - please check it's OK for you.
>
Sorry, I still reproduce it under PostgreSQL 8.2.4+ and PostgreSQL HEAD.
Here are steps to reproduce problem :
- right click on a function
- choose Properties
- change function's name
OK button will not be enabled, SQL string will not show.
--
Guillaume.
http://www.postgresqlfr.org
http://docs.postgresqlfr.org
Guillaume Lelarge wrote: > Sorry, I still reproduce it under PostgreSQL 8.2.4+ and PostgreSQL HEAD. > > Here are steps to reproduce problem : > - right click on a function > - choose Properties > - change function's name > > OK button will not be enabled, SQL string will not show. Are you sure you're running the build you think you are? Following the steps you detail above works perfectly on PostgreSQL 8.2.4 and EnterpriseDB AS 8.2 for me. I've tried C, pl/pgsql and edbspl functions (though the latter only on EDB of course). Not that it should matter, but this is SVN head of pgAdmin, running on XP. Regards, Dave.
Dave Page a écrit : > Guillaume Lelarge wrote: >> Sorry, I still reproduce it under PostgreSQL 8.2.4+ and PostgreSQL HEAD. >> >> Here are steps to reproduce problem : >> - right click on a function >> - choose Properties >> - change function's name >> >> OK button will not be enabled, SQL string will not show. > > Are you sure you're running the build you think you are? Following the > steps you detail above works perfectly on PostgreSQL 8.2.4 and > EnterpriseDB AS 8.2 for me. I've tried C, pl/pgsql and edbspl functions > (though the latter only on EDB of course). > > Not that it should matter, but this is SVN head of pgAdmin, running on XP. > OK, I think I've found a fix to Pavel's issue. To check cbLanguage, we use GetCurrentSelection. When I try to change an already defined function, GetCurrentSelection always returns -1 which is an invalid value. If I use GetGuessedSelection (with every other changes needed to use ctlComboBox instead of wxComboBox), it works great. I'm not sure this is the right way to fix this, so here is the patch. As always, comments welcomed. Regards. -- Guillaume. <!-- http://abs.traduc.org/ http://lfs.traduc.org/ http://docs.postgresqlfr.org/ --> Index: pgadmin/dlg/dlgFunction.cpp =================================================================== --- pgadmin/dlg/dlgFunction.cpp (révision 6504) +++ pgadmin/dlg/dlgFunction.cpp (copie de travail) @@ -30,7 +30,7 @@ #define stReturntype CTRL_STATIC("stReturntype") #define cbReturntype CTRL_COMBOBOX2("cbReturntype") #define stLanguage CTRL_STATIC("stLanguage") -#define cbLanguage CTRL_COMBOBOX("cbLanguage") +#define cbLanguage CTRL_COMBOBOX2("cbLanguage") #define chkSetof CTRL_CHECKBOX("chkSetof") #define stVolatility CTRL_STATIC("stVolatility") #define cbVolatility CTRL_COMBOBOX("cbVolatility") @@ -339,7 +339,7 @@ CheckValid(enable, cbReturntype->GetGuessedSelection() >= 0, _("Please select return type.")); if (!(isProcedure && connection->GetIsEdb())) - CheckValid(enable, cbLanguage->GetCurrentSelection() >= 0, _("Please select language.")); + CheckValid(enable, cbLanguage->GetGuessedSelection() >= 0, _("Please select language.")); if (isC) { Index: pgadmin/ui/dlgFunction.xrc =================================================================== --- pgadmin/ui/dlgFunction.xrc (révision 6504) +++ pgadmin/ui/dlgFunction.xrc (copie de travail) @@ -59,7 +59,7 @@ <label>Language</label> <pos>5,82d</pos> </object> - <object class="wxComboBox" name="cbLanguage"> + <object class="ctlComboBox" name="cbLanguage"> <content/> <pos>70,80d</pos> <size>135,12d</size> Index: pgadmin/ui/xrcDialogs.cpp =================================================================== --- pgadmin/ui/xrcDialogs.cpp (révision 6504) +++ pgadmin/ui/xrcDialogs.cpp (copie de travail) @@ -2565,7 +2565,7 @@ 62,10,32,32,60,47,111,98,106,101,99,116,62,10,60,47,114,101,115,111,117, 114,99,101,62,10}; -static size_t xml_res_size_13 = 13937; +static size_t xml_res_size_13 = 13938; static unsigned char xml_res_file_13[] = { 60,63,120,109,108,32,118,101,114,115,105,111,110,61,34,49,46,48,34,32,101, 110,99,111,100,105,110,103,61,34,73,83,79,45,56,56,53,57,45,49,34,63,62, @@ -2676,9 +2676,9 @@ 32,32,32,32,32,32,32,60,112,111,115,62,53,44,56,50,100,60,47,112,111,115, 62,10,32,32,32,32,32,32,32,32,32,32,32,32,32,32,60,47,111,98,106,101,99, 116,62,10,32,32,32,32,32,32,32,32,32,32,32,32,32,32,60,111,98,106,101,99, -116,32,99,108,97,115,115,61,34,119,120,67,111,109,98,111,66,111,120,34, -32,110,97,109,101,61,34,99,98,76,97,110,103,117,97,103,101,34,62,10,32, -32,32,32,32,32,32,32,32,32,32,32,32,32,32,32,60,99,111,110,116,101,110, +116,32,99,108,97,115,115,61,34,99,116,108,67,111,109,98,111,66,111,120, +34,32,110,97,109,101,61,34,99,98,76,97,110,103,117,97,103,101,34,62,10, +32,32,32,32,32,32,32,32,32,32,32,32,32,32,32,32,60,99,111,110,116,101,110, 116,47,62,10,32,32,32,32,32,32,32,32,32,32,32,32,32,32,32,32,60,112,111, 115,62,55,48,44,56,48,100,60,47,112,111,115,62,10,32,32,32,32,32,32,32, 32,32,32,32,32,32,32,32,32,60,115,105,122,101,62,49,51,53,44,49,50,100,
Guillaume Lelarge wrote: > Dave Page a écrit : >> Guillaume Lelarge wrote: >>> Sorry, I still reproduce it under PostgreSQL 8.2.4+ and PostgreSQL HEAD. >>> >>> Here are steps to reproduce problem : >>> - right click on a function >>> - choose Properties >>> - change function's name >>> >>> OK button will not be enabled, SQL string will not show. >> Are you sure you're running the build you think you are? Following the >> steps you detail above works perfectly on PostgreSQL 8.2.4 and >> EnterpriseDB AS 8.2 for me. I've tried C, pl/pgsql and edbspl functions >> (though the latter only on EDB of course). >> >> Not that it should matter, but this is SVN head of pgAdmin, running on XP. >> > > OK, I think I've found a fix to Pavel's issue. To check cbLanguage, we > use GetCurrentSelection. When I try to change an already defined > function, GetCurrentSelection always returns -1 which is an invalid > value. If I use GetGuessedSelection (with every other changes needed to > use ctlComboBox instead of wxComboBox), it works great. I'm not sure > this is the right way to fix this, so here is the patch. > > As always, comments welcomed. Hmm, sounds like it might be platform specific issue, similar to (but not the same) one Stefan fixed for me in wxMac before 2.8 was released. What OS and version of wxWidgets is this? I have no problem with the patch, so if this is reproducable with the latest wx version, please go ahead and apply. Regards, Dave.
Dave Page a écrit : > Guillaume Lelarge wrote: >[...] >> OK, I think I've found a fix to Pavel's issue. To check cbLanguage, we >> use GetCurrentSelection. When I try to change an already defined >> function, GetCurrentSelection always returns -1 which is an invalid >> value. If I use GetGuessedSelection (with every other changes needed to >> use ctlComboBox instead of wxComboBox), it works great. I'm not sure >> this is the right way to fix this, so here is the patch. >> >> As always, comments welcomed. > > Hmm, sounds like it might be platform specific issue, similar to (but > not the same) one Stefan fixed for me in wxMac before 2.8 was released. > > What OS and version of wxWidgets is this? > Kubuntu 7.04 (linux 2.6.20). wxWidgets 2.8.4 (kubuntu's package). I don't know for Pavel. > I have no problem with the patch, so if this is reproducable with the > latest wx version, please go ahead and apply. > If you're still OK, I will commit it today. Regards. -- Guillaume. <!-- http://abs.traduc.org/ http://lfs.traduc.org/ http://docs.postgresqlfr.org/ -->
Guillaume Lelarge wrote: > Dave Page a écrit : >> Guillaume Lelarge wrote: >> [...] >>> OK, I think I've found a fix to Pavel's issue. To check cbLanguage, we >>> use GetCurrentSelection. When I try to change an already defined >>> function, GetCurrentSelection always returns -1 which is an invalid >>> value. If I use GetGuessedSelection (with every other changes needed to >>> use ctlComboBox instead of wxComboBox), it works great. I'm not sure >>> this is the right way to fix this, so here is the patch. >>> >>> As always, comments welcomed. >> Hmm, sounds like it might be platform specific issue, similar to (but >> not the same) one Stefan fixed for me in wxMac before 2.8 was released. >> >> What OS and version of wxWidgets is this? >> > > Kubuntu 7.04 (linux 2.6.20). > wxWidgets 2.8.4 (kubuntu's package). > > I don't know for Pavel. > >> I have no problem with the patch, so if this is reproducable with the >> latest wx version, please go ahead and apply. >> > > If you're still OK, I will commit it today. Please do. Thanks, Dave
Dave Page a écrit : > Guillaume Lelarge wrote: >> Dave Page a écrit : >>> Guillaume Lelarge wrote: >>> [...] >>>> OK, I think I've found a fix to Pavel's issue. To check cbLanguage, we >>>> use GetCurrentSelection. When I try to change an already defined >>>> function, GetCurrentSelection always returns -1 which is an invalid >>>> value. If I use GetGuessedSelection (with every other changes needed to >>>> use ctlComboBox instead of wxComboBox), it works great. I'm not sure >>>> this is the right way to fix this, so here is the patch. >>>> >>>> As always, comments welcomed. >>> Hmm, sounds like it might be platform specific issue, similar to (but >>> not the same) one Stefan fixed for me in wxMac before 2.8 was released. >>> >>> What OS and version of wxWidgets is this? >>> >> Kubuntu 7.04 (linux 2.6.20). >> wxWidgets 2.8.4 (kubuntu's package). >> >> I don't know for Pavel. >> >>> I have no problem with the patch, so if this is reproducable with the >>> latest wx version, please go ahead and apply. >>> >> If you're still OK, I will commit it today. > > Please do. > Done. Thanks. -- Guillaume. <!-- http://abs.traduc.org/ http://lfs.traduc.org/ http://docs.postgresqlfr.org/ -->