Thread: pgAdmin III: adjust code as per new EDB AS90 functions/procedures semantics
Hi,
In EDBAS 9.0, following notable restructuring has been done with respect to functions and procedures. We need to adopt the same in the pgAdmin code base. Note that these changes make EDBAS funcs/procs behave more like native Postgres functions which is a good thing:
* To debug edb-SPL functions/procedures, the debugger was using custom EDB-protocol extension. With the latest refactorings, the normal code path which was being employed for plpgsql objects can be used. This patch checks for the version number at appropriate locations to avoid calling this custom protocol now. Obviously the current behavior has to be retained for pre 9.0 versions. (changes in debugger files. Had to also make some misc. changes to handle default parameters appropriately.)
* Procedures with or without OUT-parameters can now also be called without the EDB-protocol extension with the EXEC command. For example, if you have a procedure like "fooproc(a IN integer, b OUT integer)", you can call it with "exec fooproc(123)". The OUT value is returned as the return value, just as if it was a function call. Again this behaviour will now be the same as for normal plpgsql objects.(pgFunction.cpp)
* If an SPL-function has both an OUT-parameter and a return value, the return value is transformed into an extra OUT-parameter named "_retval_". This patch adds code to identify such a column and use its type to set the return type appropriately. (pgFunction.cpp)
Note that the changes are not as intensive as the above may sound. The protocol extension was only being used by the debugger code paths, so the changes are localised in that module. The remaining changes are in pgFunction.cpp file only. I tested the debugger changes against both AS90 and AS84 code bases. For the latter the extended protocol is getting invoked appropriately. No testing on windows though yet.
Regards,
Nikhils
In EDBAS 9.0, following notable restructuring has been done with respect to functions and procedures. We need to adopt the same in the pgAdmin code base. Note that these changes make EDBAS funcs/procs behave more like native Postgres functions which is a good thing:
* To debug edb-SPL functions/procedures, the debugger was using custom EDB-protocol extension. With the latest refactorings, the normal code path which was being employed for plpgsql objects can be used. This patch checks for the version number at appropriate locations to avoid calling this custom protocol now. Obviously the current behavior has to be retained for pre 9.0 versions. (changes in debugger files. Had to also make some misc. changes to handle default parameters appropriately.)
* Procedures with or without OUT-parameters can now also be called without the EDB-protocol extension with the EXEC command. For example, if you have a procedure like "fooproc(a IN integer, b OUT integer)", you can call it with "exec fooproc(123)". The OUT value is returned as the return value, just as if it was a function call. Again this behaviour will now be the same as for normal plpgsql objects.(pgFunction.cpp)
* If an SPL-function has both an OUT-parameter and a return value, the return value is transformed into an extra OUT-parameter named "_retval_". This patch adds code to identify such a column and use its type to set the return type appropriately. (pgFunction.cpp)
Note that the changes are not as intensive as the above may sound. The protocol extension was only being used by the debugger code paths, so the changes are localised in that module. The remaining changes are in pgFunction.cpp file only. I tested the debugger changes against both AS90 and AS84 code bases. For the latter the extended protocol is getting invoked appropriately. No testing on windows though yet.
Regards,
Nikhils
Attachment
Re: pgAdmin III: adjust code as per new EDB AS90 functions/procedures semantics
From
Dave Page
Date:
Thanks Nikhil. Are there any catalog changes with the refactoring, that change the way parameters are represented that need to be reflected elsewhere in pgFunction.cpp? Also, does anyone object to back-patching this? It's not a bug fix, but it does mean that we don't support corresponding versions of PPAS and PG in the same version of pgAdmin which seems undesirable. The patch is pretty straightforward and seems low risk. On Mon, Feb 21, 2011 at 9:17 AM, Nikhil S <nixmisc@gmail.com> wrote: > Hi, > > In EDBAS 9.0, following notable restructuring has been done with respect to > functions and procedures. We need to adopt the same in the pgAdmin code > base. Note that these changes make EDBAS funcs/procs behave more like native > Postgres functions which is a good thing: > > * To debug edb-SPL functions/procedures, the debugger was using custom > EDB-protocol extension. With the latest refactorings, the normal code path > which was being employed for plpgsql objects can be used. This patch checks > for the version number at appropriate locations to avoid calling this custom > protocol now. Obviously the current behavior has to be retained for pre 9.0 > versions. (changes in debugger files. Had to also make some misc. changes to > handle default parameters appropriately.) > > * Procedures with or without OUT-parameters can now also be called without > the EDB-protocol extension with the EXEC command. For example, if you have a > procedure like "fooproc(a IN integer, b OUT integer)", you can call it with > "exec fooproc(123)". The OUT value is returned as the return value, just as > if it was a function call. Again this behaviour will now be the same as for > normal plpgsql objects.(pgFunction.cpp) > > * If an SPL-function has both an OUT-parameter and a return value, the > return value is transformed into an extra OUT-parameter named "_retval_". > This patch adds code to identify such a column and use its type to set the > return type appropriately. (pgFunction.cpp) > > Note that the changes are not as intensive as the above may sound. The > protocol extension was only being used by the debugger code paths, so the > changes are localised in that module. The remaining changes are in > pgFunction.cpp file only. I tested the debugger changes against both AS90 > and AS84 code bases. For the latter the extended protocol is getting invoked > appropriately. No testing on windows though yet. > > Regards, > Nikhils > > > -- > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgadmin-hackers > > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hi,
Ummm, nothing that comes to mind immediately. I think pronargs does not contain OUT params for SPL objects anymore. But did not see any ill-effects of this in my limited testing.
Additionally, this patch already handles the "_retval_" parameter appropriately. So as of now dare I say things look ok, but we might do some more coding to handle any reported issues later. Won't be earth-shattering fixes though IMO...
Are there any catalog changes with the refactoring, that change the
way parameters are represented that need to be reflected elsewhere in
pgFunction.cpp?
Ummm, nothing that comes to mind immediately. I think pronargs does not contain OUT params for SPL objects anymore. But did not see any ill-effects of this in my limited testing.
Additionally, this patch already handles the "_retval_" parameter appropriately. So as of now dare I say things look ok, but we might do some more coding to handle any reported issues later. Won't be earth-shattering fixes though IMO...
Also, does anyone object to back-patching this? It's not a bug fix,
but it does mean that we don't support corresponding versions of PPAS
and PG in the same version of pgAdmin which seems undesirable. The
patch is pretty straightforward and seems low risk.
+1 from the patch-submitter (if it counts ;))
Regards,
Nikhils
Regards,
Nikhils
> --On Mon, Feb 21, 2011 at 9:17 AM, Nikhil S <nixmisc@gmail.com> wrote:
> Hi,
>
> In EDBAS 9.0, following notable restructuring has been done with respect to
> functions and procedures. We need to adopt the same in the pgAdmin code
> base. Note that these changes make EDBAS funcs/procs behave more like native
> Postgres functions which is a good thing:
>
> * To debug edb-SPL functions/procedures, the debugger was using custom
> EDB-protocol extension. With the latest refactorings, the normal code path
> which was being employed for plpgsql objects can be used. This patch checks
> for the version number at appropriate locations to avoid calling this custom
> protocol now. Obviously the current behavior has to be retained for pre 9.0
> versions. (changes in debugger files. Had to also make some misc. changes to
> handle default parameters appropriately.)
>
> * Procedures with or without OUT-parameters can now also be called without
> the EDB-protocol extension with the EXEC command. For example, if you have a
> procedure like "fooproc(a IN integer, b OUT integer)", you can call it with
> "exec fooproc(123)". The OUT value is returned as the return value, just as
> if it was a function call. Again this behaviour will now be the same as for
> normal plpgsql objects.(pgFunction.cpp)
>
> * If an SPL-function has both an OUT-parameter and a return value, the
> return value is transformed into an extra OUT-parameter named "_retval_".
> This patch adds code to identify such a column and use its type to set the
> return type appropriately. (pgFunction.cpp)
>
> Note that the changes are not as intensive as the above may sound. The
> protocol extension was only being used by the debugger code paths, so the
> changes are localised in that module. The remaining changes are in
> pgFunction.cpp file only. I tested the debugger changes against both AS90
> and AS84 code bases. For the latter the extended protocol is getting invoked
> appropriately. No testing on windows though yet.
>
> Regards,
> Nikhils
>
>
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>
>
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Re: pgAdmin III: adjust code as per new EDB AS90 functions/procedures semantics
From
Magnus Hagander
Date:
On Feb 21, 2011 3:29 PM, "Dave Page" <dpage@pgadmin.org> wrote:
>
> Thanks Nikhil.
>
> Are there any catalog changes with the refactoring, that change the
> way parameters are represented that need to be reflected elsewhere in
> pgFunction.cpp?
>
> Also, does anyone object to back-patching this? It's not a bug fix,
> but it does mean that we don't support corresponding versions of PPAS
> and PG in the same version of pgAdmin which seems undesirable.
I wasn't aware they were supposed to be? Is that new, or has it always been?
More to the point - is this the only thing needed to reach compatibility? If so, i guess we can make an exception. If not, then there is no point without doing a bunch of more patches for other things, in which case i will object...
The
> patch is pretty straightforward and seems low risk.
This is of course a prerequisite in either case? I also assume it affects only an edbas codepath?
/Magnus
Re: pgAdmin III: adjust code as per new EDB AS90 functions/procedures semantics
From
Dave Page
Date:
On Mon, Feb 21, 2011 at 3:46 PM, Magnus Hagander <magnus@hagander.net> wrote: > > On Feb 21, 2011 3:29 PM, "Dave Page" <dpage@pgadmin.org> wrote: >> >> Thanks Nikhil. >> >> Are there any catalog changes with the refactoring, that change the >> way parameters are represented that need to be reflected elsewhere in >> pgFunction.cpp? >> >> Also, does anyone object to back-patching this? It's not a bug fix, >> but it does mean that we don't support corresponding versions of PPAS >> and PG in the same version of pgAdmin which seems undesirable. > > I wasn't aware they were supposed to be? Is that new, or has it always been? It's never really come up before, hence why I'm asking :-) > More to the point - is this the only thing needed to reach compatibility? If > so, i guess we can make an exception. If not, then there is no point without > doing a bunch of more patches for other things, in which case i will > object... Compatibility; yes, I hope so. Functionality; probably not, but I'm not going to suggest we back patch for new features. I'd like for it to work without going bang, even if we don't support the latest features yet. > The >> patch is pretty straightforward and seems low risk. > > This is of course a prerequisite in either case? I also assume it affects > only an edbas codepath? That's the intention. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: pgAdmin III: adjust code as per new EDB AS90 functions/procedures semantics
From
Magnus Hagander
Date:
On Mon, Feb 21, 2011 at 17:22, Dave Page <dpage@pgadmin.org> wrote: > On Mon, Feb 21, 2011 at 3:46 PM, Magnus Hagander <magnus@hagander.net> wrote: >> >> On Feb 21, 2011 3:29 PM, "Dave Page" <dpage@pgadmin.org> wrote: >>> >>> Thanks Nikhil. >>> >>> Are there any catalog changes with the refactoring, that change the >>> way parameters are represented that need to be reflected elsewhere in >>> pgFunction.cpp? >>> >>> Also, does anyone object to back-patching this? It's not a bug fix, >>> but it does mean that we don't support corresponding versions of PPAS >>> and PG in the same version of pgAdmin which seems undesirable. >> >> I wasn't aware they were supposed to be? Is that new, or has it always been? > > It's never really come up before, hence why I'm asking :-) No, I meant is the EDBAS version <x> supposed to "match" community pg version <x>? I haven't really looked at it since years ago, where iirc edbas was somewhere halfway between pg 8.2 and 8.3, and the version number didn't actually match either one.. >> More to the point - is this the only thing needed to reach compatibility? If >> so, i guess we can make an exception. If not, then there is no point without >> doing a bunch of more patches for other things, in which case i will >> object... > > Compatibility; yes, I hope so. Functionality; probably not, but I'm > not going to suggest we back patch for new features. I'd like for it > to work without going bang, even if we don't support the latest > features yet. Oh yeah, compatibility is all we're discussing here. So in that case, I'm fine with backpatching it. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: pgAdmin III: adjust code as per new EDB AS90 functions/procedures semantics
From
Dave Page
Date:
On Mon, Feb 21, 2011 at 4:27 PM, Magnus Hagander <magnus@hagander.net> wrote: > No, I meant is the EDBAS version <x> supposed to "match" community pg > version <x>? > > I haven't really looked at it since years ago, where iirc edbas was > somewhere halfway between pg 8.2 and 8.3, and the version number > didn't actually match either one.. Oh, yeah - PPAS 8.4 is a superset of PG 8.4 features, same with 9.0. 8.3 was the only weird one. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: pgAdmin III: adjust code as per new EDB AS90 functions/procedures semantics
From
Guillaume Lelarge
Date:
Le 21/02/2011 17:27, Magnus Hagander a écrit : > On Mon, Feb 21, 2011 at 17:22, Dave Page <dpage@pgadmin.org> wrote: >> On Mon, Feb 21, 2011 at 3:46 PM, Magnus Hagander <magnus@hagander.net> wrote: >>> >>> On Feb 21, 2011 3:29 PM, "Dave Page" <dpage@pgadmin.org> wrote: >>>> >>>> Thanks Nikhil. >>>> >>>> Are there any catalog changes with the refactoring, that change the >>>> way parameters are represented that need to be reflected elsewhere in >>>> pgFunction.cpp? >>>> >>>> Also, does anyone object to back-patching this? It's not a bug fix, >>>> but it does mean that we don't support corresponding versions of PPAS >>>> and PG in the same version of pgAdmin which seems undesirable. >>> >>> I wasn't aware they were supposed to be? Is that new, or has it always been? >> >> It's never really come up before, hence why I'm asking :-) > > No, I meant is the EDBAS version <x> supposed to "match" community pg > version <x>? > > I haven't really looked at it since years ago, where iirc edbas was > somewhere halfway between pg 8.2 and 8.3, and the version number > didn't actually match either one.. > > >>> More to the point - is this the only thing needed to reach compatibility? If >>> so, i guess we can make an exception. If not, then there is no point without >>> doing a bunch of more patches for other things, in which case i will >>> object... >> >> Compatibility; yes, I hope so. Functionality; probably not, but I'm >> not going to suggest we back patch for new features. I'd like for it >> to work without going bang, even if we don't support the latest >> features yet. > > Oh yeah, compatibility is all we're discussing here. So in that case, > I'm fine with backpatching it. > I'm not sure this is a really great idea to back-patch new compatibility code. As two already agreed on back-patching, I'm fine with it. -- Guillaume http://www.postgresql.fr http://dalibo.com
Re: pgAdmin III: adjust code as per new EDB AS90 functions/procedures semantics
From
Dave Page
Date:
Thanks, patch applied to trunk and 1.12. On Mon, Feb 21, 2011 at 9:17 AM, Nikhil S <nixmisc@gmail.com> wrote: > Hi, > > In EDBAS 9.0, following notable restructuring has been done with respect to > functions and procedures. We need to adopt the same in the pgAdmin code > base. Note that these changes make EDBAS funcs/procs behave more like native > Postgres functions which is a good thing: > > * To debug edb-SPL functions/procedures, the debugger was using custom > EDB-protocol extension. With the latest refactorings, the normal code path > which was being employed for plpgsql objects can be used. This patch checks > for the version number at appropriate locations to avoid calling this custom > protocol now. Obviously the current behavior has to be retained for pre 9.0 > versions. (changes in debugger files. Had to also make some misc. changes to > handle default parameters appropriately.) > > * Procedures with or without OUT-parameters can now also be called without > the EDB-protocol extension with the EXEC command. For example, if you have a > procedure like "fooproc(a IN integer, b OUT integer)", you can call it with > "exec fooproc(123)". The OUT value is returned as the return value, just as > if it was a function call. Again this behaviour will now be the same as for > normal plpgsql objects.(pgFunction.cpp) > > * If an SPL-function has both an OUT-parameter and a return value, the > return value is transformed into an extra OUT-parameter named "_retval_". > This patch adds code to identify such a column and use its type to set the > return type appropriately. (pgFunction.cpp) > > Note that the changes are not as intensive as the above may sound. The > protocol extension was only being used by the debugger code paths, so the > changes are localised in that module. The remaining changes are in > pgFunction.cpp file only. I tested the debugger changes against both AS90 > and AS84 code bases. For the latter the extended protocol is getting invoked > appropriately. No testing on windows though yet. > > Regards, > Nikhils > > > -- > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgadmin-hackers > > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Thanks Dave!
Regards,
Nikhils
Regards,
Nikhils
On Wed, Feb 23, 2011 at 3:57 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks, patch applied to trunk and 1.12.> Hi,
>
> In EDBAS 9.0, following notable restructuring has been done with respect to
> functions and procedures. We need to adopt the same in the pgAdmin code
> base. Note that these changes make EDBAS funcs/procs behave more like native
> Postgres functions which is a good thing:
>
> * To debug edb-SPL functions/procedures, the debugger was using custom
> EDB-protocol extension. With the latest refactorings, the normal code path
> which was being employed for plpgsql objects can be used. This patch checks
> for the version number at appropriate locations to avoid calling this custom
> protocol now. Obviously the current behavior has to be retained for pre 9.0
> versions. (changes in debugger files. Had to also make some misc. changes to
> handle default parameters appropriately.)
>
> * Procedures with or without OUT-parameters can now also be called without
> the EDB-protocol extension with the EXEC command. For example, if you have a
> procedure like "fooproc(a IN integer, b OUT integer)", you can call it with
> "exec fooproc(123)". The OUT value is returned as the return value, just as
> if it was a function call. Again this behaviour will now be the same as for
> normal plpgsql objects.(pgFunction.cpp)
>
> * If an SPL-function has both an OUT-parameter and a return value, the
> return value is transformed into an extra OUT-parameter named "_retval_".
> This patch adds code to identify such a column and use its type to set the
> return type appropriately. (pgFunction.cpp)
>
> Note that the changes are not as intensive as the above may sound. The
> protocol extension was only being used by the debugger code paths, so the
> changes are localised in that module. The remaining changes are in
> pgFunction.cpp file only. I tested the debugger changes against both AS90
> and AS84 code bases. For the latter the extended protocol is getting invoked
> appropriately. No testing on windows though yet.
>
> Regards,
> Nikhils
>
>> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>
>
--Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company