WIP: Allow SQL-language functions to reference parameters by parameter name

Matthew Draper <matthew@trebex.net> writes:

Attached is a WIP patch that allows SQL-language functions to reference
their parameters by name.

It uses p_post_columnref_hook, so potentially ambiguous references
prefer the column... that seems to make the most sense, both because it
avoids a backwards incompatibility, and it conforms with SQL's usual
notion of assuming you mean the "nearest" name.

Personally I'd vote for *not* having any such dangerous semantics as
that. We should have learned better by now from plpgsql experience.
I think the best idea is to throw error for ambiguous references,
period. That means you do need ways to disambiguate in both directions.
For column references you can just qualify with the table name/alias.
If the parameter reference is intended, allow qualification with the
function name.

regards, tom lane

2011/3/25 Tom Lane <tgl@sss.pgh.pa.us>:

Matthew Draper <matthew@trebex.net> writes:

Attached is a WIP patch that allows SQL-language functions to reference
their parameters by name.

It uses p_post_columnref_hook, so potentially ambiguous references
prefer the column... that seems to make the most sense, both because it
avoids a backwards incompatibility, and it conforms with SQL's usual
notion of assuming you mean the "nearest" name.

Personally I'd vote for *not* having any such dangerous semantics as
that.  We should have learned better by now from plpgsql experience.
I think the best idea is to throw error for ambiguous references,
period.  That means you do need ways to disambiguate in both directions.
For column references you can just qualify with the table name/alias.
If the parameter reference is intended, allow qualification with the
function name.

I agree with Tom.

There can be GUC for controlling use or don't use a parameter names. I
am for GUC, because there will be a bilion conflicts. But a talk about
priorities - sql identifier or parameter is useless.

Regards

Pavel Stehule

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

2011/3/25 Tom Lane <tgl@sss.pgh.pa.us>:

I think the best idea is to throw error for ambiguous references,
period.

There can be GUC for controlling use or don't use a parameter names. I
am for GUC, because there will be a bilion conflicts. But a talk about
priorities - sql identifier or parameter is useless.

GUCs are not tremendously helpful for problems such as this. If we
actually wanted to preserve full backwards compatibility, we'd need to
think of a way to mark SQL functions per-function as to what to do.
But I don't think that's necessary. Up to now there's been relatively
little use for naming the parameters of SQL functions, so I think there
will be few conflicts in the field if we just change the behavior. The
mess and complication we have for the comparable behavior in plpgsql
seemed necessary because of the number of existing usages that would
certainly break --- but I doubt that SQL-language functions will have
anywhere near as big a problem.

regards, tom lane

2011/3/25 Tom Lane <tgl@sss.pgh.pa.us>:

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

2011/3/25 Tom Lane <tgl@sss.pgh.pa.us>:

I think the best idea is to throw error for ambiguous references,
period.

There can be GUC for controlling use or don't use a parameter names. I
am for GUC, because there will be a bilion conflicts. But a talk about
priorities - sql identifier or parameter is useless.

GUCs are not tremendously helpful for problems such as this.  If we
actually wanted to preserve full backwards compatibility, we'd need to
think of a way to mark SQL functions per-function as to what to do.
But I don't think that's necessary.  Up to now there's been relatively
little use for naming the parameters of SQL functions, so I think there
will be few conflicts in the field if we just change the behavior.  The
mess and complication we have for the comparable behavior in plpgsql
seemed necessary because of the number of existing usages that would
certainly break --- but I doubt that SQL-language functions will have
anywhere near as big a problem.

should be nice some converting tool for pg_dump or pg_upgrade. It can
dump SQL functions with only qualified identifiers.

Pavel

On Mar 25, 2011, at 4:20 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

GUCs are not tremendously helpful for problems such as this. If we
actually wanted to preserve full backwards compatibility, we'd need to
think of a way to mark SQL functions per-function as to what to do.
But I don't think that's necessary. Up to now there's been relatively
little use for naming the parameters of SQL functions, so I think there
will be few conflicts in the field if we just change the behavior.

Oh wow, I don't agree with that at all. People may name the parameters for documentation purposes, and then have things like WHERE foo = $1, foo happening also to be the name associated with $1. Boom!

In any case, I think this is 9.2 material. We need to get a beta out the door, and I emphatically think we should be focusing on resolving the issues with features already committed, not committing new ones.

...Robert

Robert Haas <robertmhaas@gmail.com> writes:

On Mar 25, 2011, at 4:20 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

But I don't think that's necessary. Up to now there's been relatively
little use for naming the parameters of SQL functions, so I think there
will be few conflicts in the field if we just change the behavior.

Oh wow, I don't agree with that at all. People may name the parameters for documentation purposes, and then have things like WHERE foo = $1, foo happening also to be the name associated with $1. Boom!

Well, maybe, but it's not like it's subtle or hard to fix.

In any case, I think this is 9.2 material.

Oh, of course. It *is* just a WIP patch, anyway.

regards, tom lane

On Mar 25, 2011, at 7:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Well, maybe, but it's not like it's subtle or hard to fix.

Depends how much of it you have. I've become very skeptical of anything that breaks pg_dump-and-reload-ability. And doubly so now that such problems also mean breaking pg_upgrade after the old cluster has been shut down.

...Robert

Robert Haas <robertmhaas@gmail.com> writes:

On Mar 25, 2011, at 7:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Well, maybe, but it's not like it's subtle or hard to fix.

Depends how much of it you have. I've become very skeptical of
anything that breaks pg_dump-and-reload-ability.

This wouldn't break pg_dump scripts, because they disable
check_function_bodies. You would get a failure on first *use*
of a function, which is something different.

Basically my concern here is that in the name of easing a short-term
conversion issue, we'll be condemning users to a future of subtle,
hard-to-find bugs due to ambiguous names. How many hundreds of
reports have we seen about the equivalent problem in plpgsql?

You could argue that the frequency of plpgsql issues was at least partly
due to having a poor choice of which way to resolve the ambiguity, but
I don't think it can be entirely blamed on that.

regards, tom lane

On Fri, Mar 25, 2011 at 8:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Mar 25, 2011, at 7:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Well, maybe, but it's not like it's subtle or hard to fix.

Depends how much of it you have. I've become very skeptical of
anything that breaks pg_dump-and-reload-ability.

This wouldn't break pg_dump scripts, because they disable
check_function_bodies.  You would get a failure on first *use*
of a function, which is something different.

Basically my concern here is that in the name of easing a short-term
conversion issue, we'll be condemning users to a future of subtle,
hard-to-find bugs due to ambiguous names.  How many hundreds of
reports have we seen about the equivalent problem in plpgsql?

You could argue that the frequency of plpgsql issues was at least partly
due to having a poor choice of which way to resolve the ambiguity, but
I don't think it can be entirely blamed on that.

As I've said before, I believe that the root cause of this problem is
that using the same syntax for variables and column names is a bad
idea in the first place. If we used $foo or ?foo or ${foo} or $.foo
or &&foo!!$#? to mean "the parameter called foo", then this would all
be a non-issue.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Tom,

Personally I'd vote for *not* having any such dangerous semantics as
that. We should have learned better by now from plpgsql experience.
I think the best idea is to throw error for ambiguous references,
period.

As a likely heavy user of this feature, I agree with Tom here. I really don't want the column being silently preferred in SQL functions, when PL/pgSQL functions are throwing an error. I'd end up spending hours debugging this.

Also, I don't understand why this would be a dump/reload issue if $1 and $2 continue to work.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
San Francisco

On Mar 25, 2011, at 9:22 PM, Joshua Berkus <josh@agliodbs.com> wrote:

Tom,

Personally I'd vote for *not* having any such dangerous semantics as
that. We should have learned better by now from plpgsql experience.
I think the best idea is to throw error for ambiguous references,
period.

As a likely heavy user of this feature, I agree with Tom here. I really don't want the column being silently preferred in SQL functions, when PL/pgSQL functions are throwing an error. I'd end up spending hours debugging this.

Also, I don't understand why this would be a dump/reload issue if $1 and $2 continue to work.

Because an identifier that previously referred unambiguously to a column might now be ambiguous, if there is a parameter with the same name.

...Robert

Robert Haas <robertmhaas@gmail.com> writes:

On Mar 25, 2011, at 9:22 PM, Joshua Berkus <josh@agliodbs.com> wrote:

Also, I don't understand why this would be a dump/reload issue if $1 and $2 continue to work.

Because an identifier that previously referred unambiguously to a column might now be ambiguous, if there is a parameter with the same name.

Yes, a function that previously worked might now throw error, if we make
ambiguous names be errors. But this is *not* a failure that would occur
during dump/reload. You'd have to actually run the function.

regards, tom lane

On Mar 25, 2011, at 9:12 PM, Robert Haas wrote:

As I've said before, I believe that the root cause of this problem is
that using the same syntax for variables and column names is a bad
idea in the first place. If we used $foo or ?foo or ${foo} or $.foo
or &&foo!!$#? to mean "the parameter called foo", then this would all
be a non-issue.

Yes *please*. Man that would make maintenance of such functions easier.

Best,

David

Robert Haas <robertmhaas@gmail.com> writes:

As I've said before, I believe that the root cause of this problem is
that using the same syntax for variables and column names is a bad
idea in the first place. If we used $foo or ?foo or ${foo} or $.foo
or &&foo!!$#? to mean "the parameter called foo", then this would all
be a non-issue.

If this were PL/perl, or PL/almost-anything-except-SQL, I could get
behind such a proposal. But it's not, it's SQL; and SQL doesn't do
things that way. SQL's idea of disambiguation is qualified names.

And even more to the point: to the extent you think that weird syntax
might be a suitable solution, you have to keep in mind that the SQL
committee could take over any such syntax at the drop of a hat.
See the recent unpleasantness concerning => ...

regards, tom lane

Robert Haas wrote:

On Mar 25, 2011, at 9:22 PM, Joshua Berkus <josh@agliodbs.com> wrote:

Tom,

Personally I'd vote for *not* having any such dangerous semantics as
that. We should have learned better by now from plpgsql experience.
I think the best idea is to throw error for ambiguous references,
period.

As a likely heavy user of this feature, I agree with Tom here. I really don't want the column being silently preferred in SQL functions, when PL/pgSQL functions are throwing an error. I'd end up spending hours debugging this.

Also, I don't understand why this would be a dump/reload issue if $1 and $2 continue to work.

Because an identifier that previously referred unambiguously to a column might now be ambiguous, if there is a parameter with the same name.

I mention 2 possible solutions here, both which involve syntax alterations, each
between the ---------- lines. I personally like the second/lower option more.

------------

Might it be reasonable, perhaps as a 9.2 feature, to add top-level-namespaces so
that one could always explicitly qualify what they are referring to?

For example, you could have the 3 "sch", "lex", "attr" (I may have missed some
useful ones).

The "sch" TLN would unambiguously refer directly to a schema object, such as a
database table.

The "lex" TLN would unambiguously refer directly to a lexical, either a
parameter of the current routine or to a lexical variable.

The "attr" TLN would unambiguously refer to a table/etc column/attribute in the
manner typical for SQL.

Use them like:

sch.foo - the table/etc foo
lex.foo - the lexical variable foo
attr.foo - the column foo

Use of these TLN are optional where there is no ambiguity.

The TLN are not reserved words, but if one has an entity named the same, then
references to it must be TLN-qualified; eg:

lex.sch
lex.lex
lex.attr

Now these are just examples. You may find a different set works better.

--------------

There are also alternate solutions.

For example, it could be mandated that lexical-scope aliases for any
data/var-like schema object are required in routines, where the aliases are
distinct from all lexical vars/params/etc, and then all SQL/code in the routines
may only refer to the schema objects by the aliases.

Effectively this makes it so that routines can no longer see non-lexical vars
but for those from parameters, and this aliasing is defining a parameter whose
argument is supplied by the DBMS automatically rather than as an explicit
routine caller argument.

That way, inside a routine body there are only lexical names for things, and so
no namespace-qualification is ever needed by the regular SQL.

Similarly, if you always think of table column names as referring to an
attribute or element of a table variable, then just reference the column
qualified by the table name (or the lexical alias thereof). Same as you do in
any other programming language. Of course, sometimes you don't have to qualify
column name references as context could make it unambiguous. Or, a shorthand
like a simple leading "." could unambiguously say you're referring to a column
of the particular table in context.

With those in place, all unqualified references are straight to lexical
variables or parameters.

And so, this is also an effective way to resolve the ambiguity and I prefer the
latter design personally.

Here's an example in quasi-PL/PgSQL:

create function myrtn (myparam integer, mytbl ::= mydb.myschema.mytbl) as
declare
myvar integer := 5;
$body$
begin
select (.mycol + myvar * myparam) as mynewcol from mytbl;
end;
$body$

Note that I've already thought through this last example as these methods of
avoiding ambiguity are loosely-speaking how my language Muldis D avoids the
problem faced by many SQL procedures.

The ".mycol" syntax specifically was inspired originally for me by Perl 6 where
the lack of something just before the "." means that the implicit topic variable
is referred to, like if you said "$_.mycol".

A Perl 6 analogy being something like:

$mytbl.map:{ .mycol + $myvar * $myparam }

aka:

$mytbl.map:{ $_.mycol + $myvar * $myparam }

--------------

-- Darren Duncan

On Mar 25, 2011, at 11:23 PM, Tom Lane wrote:

If this were PL/perl, or PL/almost-anything-except-SQL, I could get
behind such a proposal. But it's not, it's SQL; and SQL doesn't do
things that way. SQL's idea of disambiguation is qualified names.

And even more to the point: to the extent you think that weird syntax
might be a suitable solution, you have to keep in mind that the SQL
committee could take over any such syntax at the drop of a hat.
See the recent unpleasantness concerning => ...

Perhaps we could use `=>varname`. ;-P

David

2011/3/26 Darren Duncan <darren@darrenduncan.net>:

Robert Haas wrote:

On Mar 25, 2011, at 9:22 PM, Joshua Berkus <josh@agliodbs.com> wrote:

Tom,

Personally I'd vote for *not* having any such dangerous semantics as
that. We should have learned better by now from plpgsql experience.
I think the best idea is to throw error for ambiguous references,
period.

As a likely heavy user of this feature, I agree with Tom here.  I really
don't want the column being silently preferred in SQL functions, when
PL/pgSQL functions are throwing an error.  I'd end up spending hours
debugging this.

Also, I don't understand why this would be a dump/reload issue if $1 and
$2 continue to work.

Because an identifier that previously referred unambiguously to a column
might now be ambiguous, if there is a parameter with the same name.

I mention 2 possible solutions here, both which involve syntax alterations,
each between the ---------- lines.  I personally like the second/lower
option more.

------------

Might it be reasonable, perhaps as a 9.2 feature, to add
top-level-namespaces so that one could always explicitly qualify what they
are referring to?

For example, you could have the 3 "sch", "lex", "attr" (I may have missed
some useful ones).

The "sch" TLN would unambiguously refer directly to a schema object, such as
a database table.

The "lex" TLN would unambiguously refer directly to a lexical, either a
parameter of the current routine or to a lexical variable.

The "attr" TLN would unambiguously refer to a table/etc column/attribute in
the manner typical for SQL.

Use them like:

 sch.foo - the table/etc foo
 lex.foo - the lexical variable foo
 attr.foo - the column foo

Use of these TLN are optional where there is no ambiguity.

The TLN are not reserved words, but if one has an entity named the same,
then references to it must be TLN-qualified; eg:

 lex.sch
 lex.lex
 lex.attr

Now these are just examples.  You may find a different set works better.

-1

this is not based on any pattern on SQL. It's not simple, and it
introduce a reserved keywords

Regards

Pavel

On Mar 25, 2011, at 11:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

If this were PL/perl, or PL/almost-anything-except-SQL, I could get
behind such a proposal. But it's not, it's SQL; and SQL doesn't do
things that way. SQL's idea of disambiguation is qualified names.

And even more to the point: to the extent you think that weird syntax
might be a suitable solution, you have to keep in mind that the SQL
committee could take over any such syntax at the drop of a hat.
See the recent unpleasantness concerning => ...

You can't be guaranteed that they won't standardize something incompatible no matter what we do. We could choose to do it as you've proposed and they could then standardize some weird syntax - the => is a fairly relevant example of exactly that.

...Robert

Pavel Stehule wrote:

2011/3/26 Darren Duncan <darren@darrenduncan.net>:

I mention 2 possible solutions here, both which involve syntax alterations,
each between the ---------- lines. I personally like the second/lower
option more.

-1

this is not based on any pattern on SQL. It's not simple, and it
introduce a reserved keywords

Okay, here's a much simpler proposal with the most important bit of the old one.

1. In all situations where there is ambiguity such that an identifier reference
(not declaration) may be referring to either a lexical variable/parameter of the
current routine, or to the name of the table column of the contextually current
table of the current SQL statement, the ambiguity is always resolved in favor of
the lexical var/param. If I am not mistaken, that is what PL/PgSQL already does
since 9.0.

2. If an identifier reference has a leading "." then that will force it to be
interpreted as a column instead (and the code will fail if there is no such
column), and so ".colname" is a shorthand for "tablename.colname"; but like with
the old "colname" it only works when just 1 of the source tables has "colname"
else it is still ambiguous like before.

Example:

select (.mycol + myvar * myparam) as mynewcol from mytbl;

This solution is a very terse and understandable change.

There are no reserved keywords. Legacy user code has no change where there were
no conflicts before. Legacy user code has no change in the case of conflict if
it was previously resolved to favor the lexical var/param.

Legacy user code only gains a leading "." in the few places where conflict was
resolved in favor of a column name before where a same-named lexical/param existed.

So what's not to like about this?

-- Darren Duncan