Thread: pgAdmin III: the debugger does not pre-populate default values for arguments

Hi,

The debugger, when it is invoked on a function or a procedure, provides a dialogue box to fill up values for the arguments. In case the user has created them with some default values assigned to some of the parameters, then they would expect that the debugger pre-populates them with those default values. This is not happening currently.

The attached patch provides this functionality.

We check if the "argdefvals" column exists in the output for the func/proc. If it does, it gets tokenized and added to the corresponding wsArgInfo object. These values then get displayed appropriately.

One side-effect of this feature is that earlier where non-default variables appeared as empty, they will now appear with values "". This will happen only if some arguments have defvals and some don't. We could have added code to do away with "" entries, but then I thought it is possible for people to provide "" as default values too. So we can live with this I think..

Regards,
Nikhils

Le 07/02/2011 09:03, Nikhil S a écrit :
> [...]
> The debugger, when it is invoked on a function or a procedure, provides a
> dialogue box to fill up values for the arguments. In case the user has
> created them with some default values assigned to some of the parameters,
> then they would expect that the debugger pre-populates them with those
> default values. This is not happening currently.
>
> The attached patch provides this functionality.
>
> We check if the "argdefvals" column exists in the output for the func/proc.
> If it does, it gets tokenized and added to the corresponding wsArgInfo
> object. These values then get displayed appropriately.
>
> One side-effect of this feature is that earlier where non-default variables
> appeared as empty, they will now appear with values "". This will happen
> only if some arguments have defvals and some don't. We could have added code
> to do away with "" entries, but then I thought it is possible for people to
> provide "" as default values too. So we can live with this I think..
>

Seems a great idea to me.


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

On Mon, Feb 7, 2011 at 9:03 AM, Nikhil S <nixmisc@gmail.com> wrote:
> One side-effect of this feature is that earlier where non-default variables
> appeared as empty, they will now appear with values "". This will happen
> only if some arguments have defvals and some don't. We could have added code
> to do away with "" entries, but then I thought it is possible for people to
> provide "" as default values too. So we can live with this I think..
>

Even for non-strings?

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

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

Hi,

On Fri, Feb 11, 2011 at 8:51 AM, Nikhil S <nixmisc@gmail.com> wrote:
> Hi,
>
> On Thu, Feb 10, 2011 at 8:48 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> On Mon, Feb 7, 2011 at 9:03 AM, Nikhil S <nixmisc@gmail.com> wrote:
>> > One side-effect of this feature is that earlier where non-default
>> > variables
>> > appeared as empty, they will now appear with values "". This will happen
>> > only if some arguments have defvals and some don't. We could have added
>> > code
>> > to do away with "" entries, but then I thought it is possible for people
>> > to
>> > provide "" as default values too. So we can live with this I think..
>> >
>>
>> Even for non-strings?
>>
>
> Consider the following procedure:
>
> CREATE OR REPLACE PROCEDURE pass_proc(p1 IN INTEGER, p2 IN INTEGER, p3 IN
> INTEGER DEFAULT 0) IS
> BEGIN
>  dbms_output.put_line('Parameter #1 P1 = ' || p1);
> END;
>
> The pldbg_get_target_info() function call returns the following info for
> defvals for this procedure:
>
> {"","",0}
>
> Because of this the first two params will end up getting "" prepopulated.
>
> Note that if there are no default values, the the defvals column is
> completely empty. Now why we see the "" for non-default arguments - the
> issue seems to be on the server side in the contrib module calls to compute
> the default values.

We need to change that then - we can't have the missing default values
default to something that's not valid for that datatype. Change them
to null or just empty them?

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

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

>

On Fri, Feb 11, 2011 at 5:11 PM, Nikhil S <nixmisc@gmail.com> wrote:
>>
>>
>> > Note that if there are no default values, the the defvals column is
>> > completely empty. Now why we see the "" for non-default arguments - the
>> > issue seems to be on the server side in the contrib module calls to
>> > compute
>> > the default values.
>>
>> We need to change that then - we can't have the missing default values
>> default to something that's not valid for that datatype. Change them
>> to null or just empty them?
>>
>
> Hmmm, well variables can default to NULL too. The backend code tries to
> track those variables which default to NULL or to other values. For the
> rest, it defaults them to "". It then uses construct_md_array to generate
> the output string for type TEXTOID. Dunno if we can have nothing (an empty
> string) as valid TEXTOID output. I guess it's a limitation for this type.
> Let me see if we can do something here on the backend side...

Doesn't really help, as we need to ensure it works sanely with
existing servers too.

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

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

Regards,
Nikhils

On Sat, Feb 12, 2011 at 6:32 AM, Nikhil S <nixmisc@gmail.com> wrote:
>
>> > Hmmm, well variables can default to NULL too. The backend code tries to
>> > track those variables which default to NULL or to other values. For the
>> > rest, it defaults them to "". It then uses construct_md_array to
>> > generate
>> > the output string for type TEXTOID. Dunno if we can have nothing (an
>> > empty
>> > string) as valid TEXTOID output. I guess it's a limitation for this
>> > type.
>> > Let me see if we can do something here on the backend side...
>>
>> Doesn't really help, as we need to ensure it works sanely with
>> existing servers too.
>>
>
> Well it will work with only those servers which support default values for
> arguments (PG 8.4 onwards, right?).
>
> But I guess you mean, we should find a solution more on the pgadmin side
> instead of tinkering with the backend?

Yes.


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

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

Hi,

Ok. One way that I can think of is that since we have the typeoids handily available, we can check the same and see if it is of stringlike type (PGOID_TYPE_CHAR, PGOID_TYPE_NAME, PGOID_TYPE_TEXT, etc..). If yes, then we retain the "", else we ignore it. Sounds reasonable?

Regards,
Nikhils

On Mon, Feb 14, 2011 at 3:53 AM, Nikhil S <nixmisc@gmail.com> wrote:
> Hi,
>
>> > But I guess you mean, we should find a solution more on the pgadmin side
>> > instead of tinkering with the backend?
>>
>> Yes.
>>
>
> Ok. One way that I can think of is that since we have the typeoids handily
> available, we can check the same and see if it is of stringlike type
> (PGOID_TYPE_CHAR, PGOID_TYPE_NAME, PGOID_TYPE_TEXT, etc..). If yes, then we
> retain the "", else we ignore it. Sounds reasonable?

Yes.

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

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

PFA, version 2 of this patch. We now check if the type of the argument is stringlike. If yes, we retain the "", else we default to nothing.

Regards,
Nikhils

Thanks - applied.

On Mon, Feb 14, 2011 at 12:53 PM, Nikhil S <nixmisc@gmail.com> wrote:
>
>> > Ok. One way that I can think of is that since we have the typeoids
>> > handily
>> > available, we can check the same and see if it is of stringlike type
>> > (PGOID_TYPE_CHAR, PGOID_TYPE_NAME, PGOID_TYPE_TEXT, etc..). If yes, then
>> > we
>> > retain the "", else we ignore it. Sounds reasonable?
>>
>> Yes.
>>
>
> PFA, version 2 of this patch. We now check if the type of the argument is
> stringlike. If yes, we retain the "", else we default to nothing.
>
> Regards,
> Nikhils
>



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

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