Re: [pgadmin-support] Missing defaults for function arguments

On Mon, Jun 11, 2012 at 5:43 PM, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com
> wrote:

> On Mon, Jun 11, 2012 at 10:07 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> On Mon, Jun 11, 2012 at 5:32 PM, Ashesh Vashi
>> <ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>> > On Mon, Jun 11, 2012 at 6:30 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>> >>
>> >>
>> >>
>> >> On Sun, Jun 10, 2012 at 6:57 PM, Ashesh Vashi
>> >> <ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>> >>>
>> >>> Hi Dave/Guillaume,
>> >>>
>> >>> Please find the attached patch to resolve this issue.
>> >>> In the following commit - the file was modified and we were not able
>> to
>> >>> spot the issue earlier.
>> >>> a265fb2977253fce436e276320d337425639384c
>> >>>
>> >>> It can be applied to both branches - REL-1_14_0_PATCHES & master.
>> >>>
>> >>
>> >> I'm not sure I see how this will work. From what I can see of the
>> patch,
>> >> it'll use the column name proargdefvals for PPAS 8.3+ and
>> proargdefaults for
>> >> PG. However;
>> >>
>> >> - PPAS 8.3 uses proargdefvals
>> >> - PPAS 9.0 uses proargdefaults (I don't have 8.4 to hand, but I assume
>> it
>> >> took the new PG naming)
>> >> - PostgreSQL uses proargdefaults.
>> >>
>> >> So, shouldn't the code be something more like:
>> >>
>> >> wxString defCol;
>> >>
>> >> if (EdbMinimumVersion(8, 3))
>> >> {
>> >> defCol = wxT("'proargdefvals'");
>> >> }
>> >>
>> >> if (BackendMinimumVersion(8, 4))
>> >> {
>> >> defCol = wxT("'proargdefaults'");
>> >> }
>> >>
>> >> Please check PPAS 8.4, and update the patch accordingly (assuming you
>> >> agree with my comments).
>> >
>> > Only PPAS 8.3 uses, 'proargdefvals' for default arguments.
>> > PG 8.4+ uses 'proargdefaults' for it and PPAS 8.4+adopted it from PG
>> 8.4.
>> >
>> > As per your suggestion, if we set the string for defCol only for
>> particular
>> > version of database server.
>> > The variable - defCol will be empty for PG < 8.4 (not PPAS).
>> > And, the query following the variable assignment will result in to an
>> error
>> > all the time for those servers.
>>
>> OK, I didn't look at the function any further. In that case, what
>> about something like:
>>
>> wxString defCol = wxT("'proargdefaults'");
>>
>> if (EdbMinimumVersion(8, 3) && !EdbMinimumVersion(8, 4))
>> {
>> defCol = wxT("'proargdefvals'");
>> }
>>
> You're right.
> I made the mistake here.
> Here is the updated patch.
>
>
Thanks, applied - I didn't patch 1.14 as we won't have any more releases of
that branch.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company