Horton

Thread

Commits

  1. Update dummy prosrc values.

  2. Convert built-in SQL-language functions to SQL-standard-body style.

  3. Split function definitions out of system_views.sql into a new file.

  1. Converting built-in SQL functions to new style

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-04-15T23:25:39Z

    [ moving this to a new thread so as not to confuse the cfbot ]

I wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> Is there anything else we should be doing along the eat your own dogfood
>> line that don't have these security implications?

> We can still convert the initdb-created SQL functions to new style,
> since there's no security threat during initdb.  I'll make a patch
> for that soon.

Here's a draft patch that converts all the built-in and information_schema
SQL functions to new style, except for half a dozen that cannot be
converted because they use polymorphic arguments.

Leaving that remaining half-a-dozen as old style seems okay from a
security standpoint, because they are few enough and simple enough
that it's no big notational headache to make their source text 100%
search-path-proof.  I've inserted OPERATOR() notation where necessary
to make them bulletproof.

Also worth a comment perhaps is that for the functions that are being
converted, I replaced the prosrc text in pg_proc.dat with "see
system_views.sql".  I think this might reduce confusion by making
it clear that these are not the operative definitions.

One thing this patch does that's not strictly within the charter
is to give the two forms of ts_debug() pg_proc.dat entries, just
so they are more like their new neighbors.  This means they'll be
pinned where before they were not, but that seems desirable to me.

I'm pretty confident the conversion is accurate, because I used \sf
to generate the text for the replacement definitions.  So I think
this is committable, though review is welcome.

One thing I was wondering about, but did not pull the trigger on
here, is whether to split off the function-related stuff in
system_views.sql into a new file "system_functions.sql", as has
long been speculated about by the comments in system_views.sql.
I think it is time to do this because

(a) The function stuff now amounts to a full third of the file.

(b) While the views made by system_views.sql are intentionally
not pinned, the function-related commands are messing with
pre-existing objects that *are* pinned.  This seems quite
confusing to me, and it might interfere with the intention that
you could reload the system view definitions using this file.

Thoughts?

			regards, tom lane

  2. Re: Converting built-in SQL functions to new style

    Justin Pryzby <pryzby@telsasoft.com> — 2021-04-16T02:07:38Z

    On Thu, Apr 15, 2021 at 07:25:39PM -0400, Tom Lane wrote:
> One thing I was wondering about, but did not pull the trigger on
> here, is whether to split off the function-related stuff in
> system_views.sql into a new file "system_functions.sql"

+1

-- 
Justin

  3. Re: Converting built-in SQL functions to new style

    Noah Misch <noah@leadboat.com> — 2021-04-16T08:30:58Z

    On Thu, Apr 15, 2021 at 07:25:39PM -0400, Tom Lane wrote:
> Here's a draft patch that converts all the built-in and information_schema
> SQL functions to new style, except for half a dozen that cannot be
> converted because they use polymorphic arguments.

This patch looks good.

> One thing I was wondering about, but did not pull the trigger on
> here, is whether to split off the function-related stuff in
> system_views.sql into a new file "system_functions.sql", as has
> long been speculated about by the comments in system_views.sql.
> I think it is time to do this because
> 
> (a) The function stuff now amounts to a full third of the file.

Fair.

> (b) While the views made by system_views.sql are intentionally
> not pinned, the function-related commands are messing with
> pre-existing objects that *are* pinned.  This seems quite
> confusing to me, and it might interfere with the intention that
> you could reload the system view definitions using this file.

I'm not aware of that causing a problem.  Currently, the views give a few
errors, and the functions do not.