WIP: default values for function parameters

Pavel Stehule wrote:

I have problem with sending patch, so I am send link
http://www.pgsql.cz/patches/defaults.diff.gz

Example:
postgres=# create function fx(a int, b int default 30, c int default 40)

Could you explain why you store the default expressions in a new posexpr
type rather than in an array of text (compare pg_attrdef.adbin)?

2008/11/24 Peter Eisentraut <peter_e@gmx.net>:

Pavel Stehule wrote:

I have problem with sending patch, so I am send link
http://www.pgsql.cz/patches/defaults.diff.gz

Example:
postgres=# create function fx(a int, b int default 30, c int default 40)

Could you explain why you store the default expressions in a new posexpr
type rather than in an array of text (compare pg_attrdef.adbin)?

I would to implement named params - and there expressions, that are
used as default params, should not be continual. I don't store params
as array of text because I would to eliminate repeated expression's
parsing. So I use similar machanism used for rules or views.

Pavel

"Pavel Stehule" <pavel.stehule@gmail.com> writes:

2008/11/24 Peter Eisentraut <peter_e@gmx.net>:

Could you explain why you store the default expressions in a new posexpr
type rather than in an array of text (compare pg_attrdef.adbin)?

I would to implement named params - and there expressions, that are
used as default params, should not be continual. I don't store params
as array of text because I would to eliminate repeated expression's
parsing. So I use similar machanism used for rules or views.

Say again? The representation Peter is suggesting *is* what is used
in rules and views. If you've re-invented that wheel, undo it.

regards, tom lane

2008/11/24 Tom Lane <tgl@sss.pgh.pa.us>:

"Pavel Stehule" <pavel.stehule@gmail.com> writes:

2008/11/24 Peter Eisentraut <peter_e@gmx.net>:

Could you explain why you store the default expressions in a new posexpr
type rather than in an array of text (compare pg_attrdef.adbin)?

I would to implement named params - and there expressions, that are
used as default params, should not be continual. I don't store params
as array of text because I would to eliminate repeated expression's
parsing. So I use similar machanism used for rules or views.

Say again? The representation Peter is suggesting *is* what is used
in rules and views. If you've re-invented that wheel, undo it.

Then I am blind. I store serialised transformed expression, but if
better solution exists, then I'll use it.

regards
Pavel Stehule

On Monday 24 November 2008 11:40:31 Pavel Stehule wrote:

I would to implement named params - and there expressions, that are
used as default params, should not be continual. I don't store params
as array of text because I would to eliminate repeated expression's
parsing. So I use similar machanism used for rules or views.

You mean you want to avoid repeated parsing of expressions in case the same
expression is used as a default value for several parameters? How common
would that be?

2008/11/24 Peter Eisentraut <peter_e@gmx.net>:

On Monday 24 November 2008 11:40:31 Pavel Stehule wrote:

I would to implement named params - and there expressions, that are
used as default params, should not be continual. I don't store params
as array of text because I would to eliminate repeated expression's
parsing. So I use similar machanism used for rules or views.

You mean you want to avoid repeated parsing of expressions in case the same
expression is used as a default value for several parameters? How common
would that be?

no - I am reading default parameters in call statement parsing.
Default parameters are implemented similar to variadic functions - so
no changes on PL part - all changes are on caller part.

Pavel

Pavel Stehule escribi�:

2008/11/24 Tom Lane <tgl@sss.pgh.pa.us>:

Say again? The representation Peter is suggesting *is* what is used
in rules and views. If you've re-invented that wheel, undo it.

Then I am blind. I store serialised transformed expression, but if
better solution exists, then I'll use it.

Seem to me you just want to store the output of nodeToString.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

On Monday 24 November 2008 23:21:15 Pavel Stehule wrote:

You mean you want to avoid repeated parsing of expressions in case the
same expression is used as a default value for several parameters? How
common would that be?

no - I am reading default parameters in call statement parsing.
Default parameters are implemented similar to variadic functions - so
no changes on PL part - all changes are on caller part.

Then I don't understand why you need this special data type instead of using
an array of text with nulls for parameters without default.

2008/11/24 Peter Eisentraut <peter_e@gmx.net>:

On Monday 24 November 2008 23:21:15 Pavel Stehule wrote:

You mean you want to avoid repeated parsing of expressions in case the
same expression is used as a default value for several parameters? How
common would that be?

no - I am reading default parameters in call statement parsing.
Default parameters are implemented similar to variadic functions - so
no changes on PL part - all changes are on caller part.

Then I don't understand why you need this special data type instead of using
an array of text with nulls for parameters without default.

I expect some overhead with classic array - but this overhead will be
small and array of text with nulls is better variant, Tomorrow I'll
send updated version.

Regards
Pavel Stehule

Peter Eisentraut <peter_e@gmx.net> writes:

On Monday 24 November 2008 23:21:15 Pavel Stehule wrote:

Default parameters are implemented similar to variadic functions - so
no changes on PL part - all changes are on caller part.

Then I don't understand why you need this special data type instead of using
an array of text with nulls for parameters without default.

I'm not even sure you need to store any nulls.  We're going to require
defaults to be provided for the last N parameters consecutively, right?
So that's just what the array contents are.  Or maybe it's not an array
at all but a single text item containing the representation of a List
--- compare the storage of index expressions.  There shouldn't be any
need to read the contents of the value during function resolution;
an appropriate representation will have the number of non-defaultable
parameters stored as a separate integer column.

regards, tom lane

2008/11/25 Tom Lane <tgl@sss.pgh.pa.us>:

Peter Eisentraut <peter_e@gmx.net> writes:

On Monday 24 November 2008 23:21:15 Pavel Stehule wrote:

Default parameters are implemented similar to variadic functions - so
no changes on PL part - all changes are on caller part.

Then I don't understand why you need this special data type instead of using
an array of text with nulls for parameters without default.

I'm not even sure you need to store any nulls.  We're going to require
defaults to be provided for the last N parameters consecutively, right?
So that's just what the array contents are.  Or maybe it's not an array
at all but a single text item containing the representation of a List
--- compare the storage of index expressions.  There shouldn't be any
need to read the contents of the value during function resolution;
an appropriate representation will have the number of non-defaultable
parameters stored as a separate integer column.

this can be the most simple solution, I used special datatype because
a) I am afraid add more columns to system tables, b) I dislike
serialisation into text type, because simple select from this values
returns some "strange" values. But maybe I am thinking and searching
to much complicate solutions. I'll try to simplify patch.

Regards
Pavel Stehule

Hello

I am sending actualized versions - I accepted Tom's comments - default
expressions are serialised List stored in text field.

Regards
Pavel Stehule

2008/11/25 Tom Lane <tgl@sss.pgh.pa.us>:

On Thursday 27 November 2008 00:14:19 Pavel Stehule wrote:

I am sending actualized versions - I accepted Tom's comments - default
expressions are serialised List stored in text field.

OK, this is looking pretty good.

There is a structural problem that we need to address. With your patch,
pg_dump produces something like this:

CREATE FUNCTION foo(a integer = 1, b integer = 2, c integer = 3) RETURNS
integer
LANGUAGE sql
AS $_$ SELECT $1 + $2 + $3; $_$;

ALTER FUNCTION public.foo(a integer = 1, b integer = 2, c integer = 3) OWNER
TO peter;

The second command is rejected because default values are only accepted in
CREATE FUNCTION.

There are two ways to fix this, both having some validity:

1. We create a second version of pg_get_function_arguments() that produces
arguments without default values decoration. This is probably the
technically sound thing to do.

2. We accept the default values specification and ignore it silently. Note
that we already silently ignore the argument names. ALTER FUNCTION foo(a
int, b int) will also act on a function defined as foo(x int, y int).

Comments?

2008/11/30 Peter Eisentraut <peter_e@gmx.net>:

On Thursday 27 November 2008 00:14:19 Pavel Stehule wrote:

I am sending actualized versions - I accepted Tom's comments - default
expressions are serialised List stored in text field.

OK, this is looking pretty good.

There is a structural problem that we need to address. With your patch,
pg_dump produces something like this:

CREATE FUNCTION foo(a integer = 1, b integer = 2, c integer = 3) RETURNS
integer
LANGUAGE sql
AS $_$ SELECT $1 + $2 + $3; $_$;

ALTER FUNCTION public.foo(a integer = 1, b integer = 2, c integer = 3) OWNER
TO peter;

The second command is rejected because default values are only accepted in
CREATE FUNCTION.

There are two ways to fix this, both having some validity:

1. We create a second version of pg_get_function_arguments() that produces
arguments without default values decoration. This is probably the
technically sound thing to do.

2. We accept the default values specification and ignore it silently. Note
that we already silently ignore the argument names. ALTER FUNCTION foo(a
int, b int) will also act on a function defined as foo(x int, y int).

if this variant is possible, then will be simply implemented

regard
Pavel

Peter Eisentraut <peter_e@gmx.net> writes:

There are two ways to fix this, both having some validity:

1. We create a second version of pg_get_function_arguments() that produces
arguments without default values decoration. This is probably the
technically sound thing to do.

Yes. I think that the argument for allowing parameter names in commands
like ALTER FUNCTION is that the user might consider them part of the
function's identity. This can hardly be claimed for default values.

Also, there's a third possibility: we could revert the decision to allow
pg_dump to depend on pg_get_function_arguments in the first place. That
was really the lazy man's approach to begin with. The more we allow
pg_dump to depend on backend functions that work in a SnapshotNow world,
the more risk we have of producing inconsistent dumps.

regards, tom lane

On Nov 30, 2008, at 6:49 PM, Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

There are two ways to fix this, both having some validity:

1. We create a second version of pg_get_function_arguments() that
produces
arguments without default values decoration. This is probably the
technically sound thing to do.

Yes. I think that the argument for allowing parameter names in
commands
like ALTER FUNCTION is that the user might consider them part of the
function's identity. This can hardly be claimed for default values.

Agreed, default values should not be a part of function signatures,
although it might be nice if ALTER FUNCTION to allow default values to
be changed.

Best,

David

2008/11/30 Tom Lane <tgl@sss.pgh.pa.us>:

Peter Eisentraut <peter_e@gmx.net> writes:

There are two ways to fix this, both having some validity:

1. We create a second version of pg_get_function_arguments() that produces
arguments without default values decoration. This is probably the
technically sound thing to do.

I'll prepare new patch with this change.

Yes. I think that the argument for allowing parameter names in commands
like ALTER FUNCTION is that the user might consider them part of the
function's identity. This can hardly be claimed for default values.

Also, there's a third possibility: we could revert the decision to allow
pg_dump to depend on pg_get_function_arguments in the first place. That
was really the lazy man's approach to begin with. The more we allow
pg_dump to depend on backend functions that work in a SnapshotNow world,
the more risk we have of producing inconsistent dumps.

I don't understand well. Transactions is spanish village for me. So
there will be some finalizing necessary from You or Peter.

Regards
Pavel Stehule

2008/11/30 Peter Eisentraut <peter_e@gmx.net>:

On Thursday 27 November 2008 00:14:19 Pavel Stehule wrote:

I am sending actualized versions - I accepted Tom's comments - default
expressions are serialised List stored in text field.

OK, this is looking pretty good.

There is a structural problem that we need to address. With your patch,
pg_dump produces something like this:

CREATE FUNCTION foo(a integer = 1, b integer = 2, c integer = 3) RETURNS
integer
LANGUAGE sql
AS $_$ SELECT $1 + $2 + $3; $_$;

ALTER FUNCTION public.foo(a integer = 1, b integer = 2, c integer = 3) OWNER
TO peter;

The second command is rejected because default values are only accepted in
CREATE FUNCTION.

There are two ways to fix this, both having some validity:

1. We create a second version of pg_get_function_arguments() that produces
arguments without default values decoration. This is probably the
technically sound thing to do.

I did it. new version is attached

Regards
Pavel Stehule

It's committed.