Thread: default parameters for built-in functions (was Re: Documentation Update: Document pg_start_backup checkpoint behavior)

I wrote:
> Bruce suggested what seemed like an excellent idea, which is to make
> this self-documenting using the new default-arguments feature ---
> it'll look something like this in \df:

>                              List of functions
>  Schema | Name | Result data type |          Argument data types           
> --------+------+------------------+----------------------------------------
>  public | foo  | integer          | label text, fast boolean DEFAULT false

On poking into this, it's a bit uglier to do than I first thought.
pg_proc.proargdefaults has to be the text representation of an
expression tree.  This means that to do it directly in pg_proc.h
would take writing something like

({CONST :consttype 16 :consttypmod -1 :constlen 1 :constbyval true :constisnull false :location -1 :constvalue 1 [ 0 0
00 ]})
 

Ugly as that is, it gets worse fast: the constvalue representation is
machine-dependent, both as to width and endianness.  And maintaining
such things in the face of expression tree changes would be no fun
either.

So I think that's out.  The only alternative that comes to mind is
to have initdb issue an additional SQL command to establish the default
after the bootstrap phase; that is, something like

CREATE OR REPLACE FUNCTION pg_start_backup(label text, fast boolean DEFAULT false) RETURNS text LANGUAGE internal
STRICTAS 'start_backup';
 

in system_views.sql or some such place.

Or we could just not bother with using a default here (ie, go back to
declaring two pg_proc entries).  It's not buying us all that much.

Comments?
        regards, tom lane

Tom Lane wrote:

> The only alternative that comes to mind is
> to have initdb issue an additional SQL command to establish the default
> after the bootstrap phase; that is, something like
> 
> CREATE OR REPLACE FUNCTION
>   pg_start_backup(label text, fast boolean DEFAULT false)
>   RETURNS text LANGUAGE internal STRICT AS 'start_backup';
> 
> in system_views.sql or some such place.

I think it's worth having this, perhaps as a new SQL file to be called
by initdb.  I'm sure we can come up with new system functions for which
it's going to be useful to have default values.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane wrote:
>> The only alternative that comes to mind is
>> to have initdb issue an additional SQL command to establish the default
>> after the bootstrap phase; that is, something like
>> 
>> CREATE OR REPLACE FUNCTION
>> pg_start_backup(label text, fast boolean DEFAULT false)
>> RETURNS text LANGUAGE internal STRICT AS 'start_backup';
>> 
>> in system_views.sql or some such place.

> I think it's worth having this, perhaps as a new SQL file to be called
> by initdb.  I'm sure we can come up with new system functions for which
> it's going to be useful to have default values.

Yeah, I thought someone would raise the "there will be more soon"
argument ;-).

The reason I suggested system_views.sql is we already have one function
in there that was too unwieldy to code directly in pg_proc.h.  At some
point it might be worth breaking the file into two parts, but I won't
bother with that right now.
        regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> writes:

> So I think that's out.  The only alternative that comes to mind is
> to have initdb issue an additional SQL command to establish the default
> after the bootstrap phase; that is, something like
>
> CREATE OR REPLACE FUNCTION
>   pg_start_backup(label text, fast boolean DEFAULT false)
>   RETURNS text LANGUAGE internal STRICT AS 'start_backup';
>
> in system_views.sql or some such place.

Well, not that this is appropriate at this moment, but I had been considering
proposing we do this to most of pg_proc and pg_operator. It's really quite a
pain to maintain these files manually, especially pg_operator.

--  Gregory Stark EnterpriseDB          http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production
Tuning

Gregory Stark <stark@enterprisedb.com> writes:
> Well, not that this is appropriate at this moment, but I had been considering
> proposing we do this to most of pg_proc and pg_operator. It's really quite a
> pain to maintain these files manually, especially pg_operator.

You won't get far with doing it to pg_proc: internal functions *have to*
have entries in there, else the fmgrtab infrastructure for them doesn't
get created.  (Yeah, I suppose there are other ways to drive that, but
the fact remains that they need more than just a SQL command.)
        regards, tom lane

2009/4/7 Tom Lane <tgl@sss.pgh.pa.us>:
> I wrote:
>> Bruce suggested what seemed like an excellent idea, which is to make
>> this self-documenting using the new default-arguments feature ---
>> it'll look something like this in \df:
>
>>                              List of functions
>>  Schema | Name | Result data type |          Argument data types
>> --------+------+------------------+----------------------------------------
>>  public | foo  | integer          | label text, fast boolean DEFAULT false
>
> On poking into this, it's a bit uglier to do than I first thought.
> pg_proc.proargdefaults has to be the text representation of an
> expression tree.  This means that to do it directly in pg_proc.h
> would take writing something like
>
> ({CONST :consttype 16 :consttypmod -1 :constlen 1 :constbyval true :constisnull false :location -1 :constvalue 1 [ 0
00 0 ]}) 
>
> Ugly as that is, it gets worse fast: the constvalue representation is
> machine-dependent, both as to width and endianness.  And maintaining
> such things in the face of expression tree changes would be no fun
> either.
>
> So I think that's out.  The only alternative that comes to mind is
> to have initdb issue an additional SQL command to establish the default
> after the bootstrap phase; that is, something like
>
> CREATE OR REPLACE FUNCTION
>  pg_start_backup(label text, fast boolean DEFAULT false)
>  RETURNS text LANGUAGE internal STRICT AS 'start_backup';
>
> in system_views.sql or some such place.
>
> Or we could just not bother with using a default here (ie, go back to
> declaring two pg_proc entries).  It's not buying us all that much.
>
> Comments?
>

I like this idea - it should help for some spec types too

regards
Pavel

>                        regards, tom lane
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

On Tuesday 07 April 2009 03:36:43 Tom Lane wrote:
> You won't get far with doing it to pg_proc: internal functions *have to*
> have entries in there, else the fmgrtab infrastructure for them doesn't
> get created.  (Yeah, I suppose there are other ways to drive that, but
> the fact remains that they need more than just a SQL command.)

What is the purpose of fmgrtab anyway?  Is it speed (how much) or some 
bootstrapping issue (in which case converting "most" as proposed by Greg would 
still work)?

Peter Eisentraut <peter_e@gmx.net> writes:
> What is the purpose of fmgrtab anyway?

It's so we can find the addresses of "internal" functions to call them.
        regards, tom lane

On Thursday 09 April 2009 02:24:53 Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > What is the purpose of fmgrtab anyway?
>
> It's so we can find the addresses of "internal" functions to call them.

Ah yes of course.  But then the table can just as well be built by something 
based on rgrep PG_FUNCTION_ARGS src/backend or something like that.  Which of 
course wasn't possible 10 years ago.