Re: [pgadmin-hackers] [pgadmin-support] Missing defaults for function arguments
On Sun, Jun 10, 2012 at 6:57 PM, Ashesh Vashi 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).
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Re: [pgadmin-hackers] [pgadmin-support] Missing defaults for function arguments
On Mon, Jun 11, 2012 at 6:30 PM, Dave Page wrote:
>
>
> On Sun, Jun 10, 2012 at 6:57 PM, Ashesh Vashi <
> ashesh.va...@enterprisedb.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.
--
Thanks & Regards,
Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
Re: [pgadmin-hackers] [pgadmin-support] Missing defaults for function arguments
On Mon, Jun 11, 2012 at 5:32 PM, Ashesh Vashi
wrote:
> On Mon, Jun 11, 2012 at 6:30 PM, Dave Page wrote:
>>
>>
>>
>> On Sun, Jun 10, 2012 at 6:57 PM, Ashesh Vashi
>> 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'");
}
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Re: [pgadmin-hackers] [pgadmin-support] Missing defaults for function arguments
On Mon, Jun 11, 2012 at 10:07 PM, Dave Page wrote:
> On Mon, Jun 11, 2012 at 5:32 PM, Ashesh Vashi
> wrote:
> > On Mon, Jun 11, 2012 at 6:30 PM, Dave Page wrote:
> >>
> >>
> >>
> >> On Sun, Jun 10, 2012 at 6:57 PM, Ashesh Vashi
> >> 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 & Regards,
Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
[pgadmin-hackers] pgAdmin III commit: Fix function default value handling.
Fix function default value handling. Branch -- master Details --- http://git.postgresql.org/gitweb?p=pgadmin3.git;a=commitdiff;h=ba710f87bbea42e28e2f4f52d1edb5a6393de117 Author: Ashesh Vashi Modified Files -- pgadmin/db/pgConn.cpp |5 - 1 files changed, 4 insertions(+), 1 deletions(-) -- Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers
Re: [pgadmin-hackers] [pgadmin-support] Missing defaults for function arguments
On Mon, Jun 11, 2012 at 5:43 PM, Ashesh Vashi wrote:
> On Mon, Jun 11, 2012 at 10:07 PM, Dave Page wrote:
>
>> On Mon, Jun 11, 2012 at 5:32 PM, Ashesh Vashi
>> wrote:
>> > On Mon, Jun 11, 2012 at 6:30 PM, Dave Page wrote:
>> >>
>> >>
>> >>
>> >> On Sun, Jun 10, 2012 at 6:57 PM, Ashesh Vashi
>> >> 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
