Обсуждение: Odd procedure resolution

Поиск
Список
Период
Сортировка

Odd procedure resolution

От
Ashutosh Bapat
Дата:
Hi,
Consider following scenario

create function foo(a int) returns integer as $$begin return a; end;
$$ language plpgsql;
create procedure foo(a float) as $$begin end; $$ language plpgsql;
call foo(1);
psql:proc_func_resolution.sql:8: ERROR:  foo(integer) is not a procedure
LINE 1: call foo(1);
             ^
HINT:  To call a function, use SELECT.

to me the error message looks confusing. I am using CALL, which means
I am trying to invoke a "procedure" not a "function" and there exists
one which can be invoked. If I drop function foo() and try call again,
it succeeds.

drop function foo(a int);
DROP FUNCTION
call foo(1);
CALL

Functions and Procedures are two different objects and we enforce
different methods to invoke those, SELECT and CALL resp. So, we should
be able to filter out one or the other and try to find best candidate
of a given kind.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


Re: Odd procedure resolution

От
Ashutosh Bapat
Дата:
Incidently the fix looks quite simple. See patch attached.

With this patch we have a diffs in create_procedure test like
  CALL random();  -- error
! ERROR:  random() is not a procedure
  LINE 1: CALL random();
               ^
! HINT:  To call a function, use SELECT.
  CREATE FUNCTION cp_testfunc1(a int) RETURNS int LANGUAGE SQL AS $$
SELECT a $$;
  CREATE TABLE cp_test (a int, b text);
  CREATE PROCEDURE ptest1(x text)
--- 4,13 ----
               ^
  HINT:  No function matches the given name and argument types. You
might need to add explicit type casts.
  CALL random();  -- error
! ERROR:  function random() does not exist
  LINE 1: CALL random();
               ^
! HINT:  No function matches the given name and argument types. You
might need to add explicit type casts.
  CREATE FUNCTION cp_testfunc1(a int) RETURNS int LANGUAGE SQL AS $$
SELECT a $$;
  CREATE TABLE cp_test (a int, b text);
  CREATE PROCEDURE ptest1(x text)

If we replace "function" with "procedure" the new error messages read
"procedure random() does not exist" "No procedure matches the given
...". Those messages look better than "random() is not a procedure".

But I haven't fixed the error messages in this patch. I need to first
see if the changes are acceptable.


On Fri, Mar 23, 2018 at 3:53 PM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
> Hi,
> Consider following scenario
>
> create function foo(a int) returns integer as $$begin return a; end;
> $$ language plpgsql;
> create procedure foo(a float) as $$begin end; $$ language plpgsql;
> call foo(1);
> psql:proc_func_resolution.sql:8: ERROR:  foo(integer) is not a procedure
> LINE 1: call foo(1);
>              ^
> HINT:  To call a function, use SELECT.
>
> to me the error message looks confusing. I am using CALL, which means
> I am trying to invoke a "procedure" not a "function" and there exists
> one which can be invoked. If I drop function foo() and try call again,
> it succeeds.
>
> drop function foo(a int);
> DROP FUNCTION
> call foo(1);
> CALL
>
> Functions and Procedures are two different objects and we enforce
> different methods to invoke those, SELECT and CALL resp. So, we should
> be able to filter out one or the other and try to find best candidate
> of a given kind.
>
> --
> Best Wishes,
> Ashutosh Bapat
> EnterpriseDB Corporation
> The Postgres Database Company



-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Вложения

Re: Odd procedure resolution

От
Tom Lane
Дата:
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
> Incidently the fix looks quite simple. See patch attached.

ISTM this patch effectively proposes to make procedures have their own
namespace yet still live in pg_proc.  That is the worst of all possible
worlds IMO.  Somewhere early in this patch series, I complained that
procedures should be in a different namespace and therefore not be kept
in pg_proc but in some new catalog.  That argument was rejected on the
grounds that SQL requires them to be in the same namespace, which I
wasn't particularly sold on, but that's where we are.  If they are in
the same namespace, though, we have to live with the consequences of
that, including ambiguity.  Otherwise there will soon be questions
like "well, why can't I create both function foo(int) and procedure
foo(int), seeing that there's no question which of them a particular
statement intends to call?".

            regards, tom lane


Re: Odd procedure resolution

От
Ashutosh Bapat
Дата:
On Fri, Mar 23, 2018 at 7:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
>> Incidently the fix looks quite simple. See patch attached.
>
> ISTM this patch effectively proposes to make procedures have their own
> namespace yet still live in pg_proc.  That is the worst of all possible
> worlds IMO.  Somewhere early in this patch series, I complained that
> procedures should be in a different namespace and therefore not be kept
> in pg_proc but in some new catalog.  That argument was rejected on the
> grounds that SQL requires them to be in the same namespace, which I
> wasn't particularly sold on, but that's where we are.  If they are in
> the same namespace, though, we have to live with the consequences of
> that, including ambiguity.  Otherwise there will soon be questions
> like "well, why can't I create both function foo(int) and procedure
> foo(int), seeing that there's no question which of them a particular
> statement intends to call?".
>

That question did cross my mind and I think that's a valid question.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


Re: Odd procedure resolution

От
Robert Haas
Дата:
On Fri, Mar 23, 2018 at 10:42 AM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
> On Fri, Mar 23, 2018 at 7:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
>>> Incidently the fix looks quite simple. See patch attached.
>>
>> ISTM this patch effectively proposes to make procedures have their own
>> namespace yet still live in pg_proc.  That is the worst of all possible
>> worlds IMO.  Somewhere early in this patch series, I complained that
>> procedures should be in a different namespace and therefore not be kept
>> in pg_proc but in some new catalog.  That argument was rejected on the
>> grounds that SQL requires them to be in the same namespace, which I
>> wasn't particularly sold on, but that's where we are.  If they are in
>> the same namespace, though, we have to live with the consequences of
>> that, including ambiguity.  Otherwise there will soon be questions
>> like "well, why can't I create both function foo(int) and procedure
>> foo(int), seeing that there's no question which of them a particular
>> statement intends to call?".
>
> That question did cross my mind and I think that's a valid question.

I agree, but I'm not sure it settles the issue.  If you hand somebody
a plate and a slice of pizza and say "eat this", you expect them to
understand that they should try to eat the pizza, not the plate.  You
expect this because virtually all human beings over the age of two
understand that pizza is eatable and plates are not.  It is similar
reasonable to expect that when the database is asked to call one of
two things, one of which can be called and the other of which cannot,
it might decide to try calling the one that can be called rather than
the one that can't.  I think we need procedures and functions to live
in the same namespace because otherwise DROP ROUTINE foo(int) could
find two things equally worthy of being dropped, and we don't want
that to happen (leaving aside the question of whether DROP ROUTINE is
a good idea in the first place).  That does not mean -- to me anyway
-- that we've got to make CALL latch onto a function when a procedure
is available.

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


Re: Odd procedure resolution

От
Tom Lane
Дата:
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Mar 23, 2018 at 10:42 AM, Ashutosh Bapat
> <ashutosh.bapat@enterprisedb.com> wrote:
>> On Fri, Mar 23, 2018 at 7:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> ISTM this patch effectively proposes to make procedures have their own
>>> namespace yet still live in pg_proc.  That is the worst of all possible
>>> worlds IMO.

> ... That does not mean -- to me anyway
> -- that we've got to make CALL latch onto a function when a procedure
> is available.

My opinion remains unchanged.  If you're unhappy about the system
confusing procedure foo(int) and function foo(real), maybe the answer
is to not overload the name "foo" with such enthusiasm.  But putting
kluges into (some of) the lookup rules is just going to lead to its
own problems and surprising results.

In particular, I dislike the idea that this patch would make routine
names appear unique in some cases when they do not in others.
Overloading is complicated/confusing enough without that.

BTW, we seem to have some confusion here already:

regression=# create function foo(int) returns int as 'select $1' language sql;
CREATE FUNCTION
regression=# create procedure foo(text) as 'select $1' language sql;
CREATE PROCEDURE
regression=# drop function foo;
ERROR:  function name "foo" is not unique
HINT:  Specify the argument list to select the function unambiguously.
regression=# drop routine foo;
ERROR:  function name "foo" is not unique
HINT:  Specify the argument list to select the function unambiguously.
regression=# drop procedure foo;
ERROR:  could not find a procedure named "foo"

The first two errors are what I'd expect, but why is the third
different?

            regards, tom lane


Re: Odd procedure resolution

От
Robert Haas
Дата:
On Wed, May 16, 2018 at 3:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> My opinion remains unchanged.  If you're unhappy about the system
> confusing procedure foo(int) and function foo(real), maybe the answer
> is to not overload the name "foo" with such enthusiasm.  But putting
> kluges into (some of) the lookup rules is just going to lead to its
> own problems and surprising results.
>
> In particular, I dislike the idea that this patch would make routine
> names appear unique in some cases when they do not in others.
> Overloading is complicated/confusing enough without that.

I am not endorsing the patch and haven't looked at it, but I don't buy
the idea that having CALL prefer procedures and SELECT functions would
confuse people more than what we've got already.  As we have it,
creating an uncallable object can make CALL fail, which is certainly a
POLA violation.  You might be be able to convince me that it's better
than the alternatives, but it can't possibly be *good*.

> BTW, we seem to have some confusion here already:
>
> regression=# create function foo(int) returns int as 'select $1' language sql;
> CREATE FUNCTION
> regression=# create procedure foo(text) as 'select $1' language sql;
> CREATE PROCEDURE
> regression=# drop function foo;
> ERROR:  function name "foo" is not unique
> HINT:  Specify the argument list to select the function unambiguously.
> regression=# drop routine foo;
> ERROR:  function name "foo" is not unique
> HINT:  Specify the argument list to select the function unambiguously.
> regression=# drop procedure foo;
> ERROR:  could not find a procedure named "foo"
>
> The first two errors are what I'd expect, but why is the third
> different?

Good question.

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


Re: Odd procedure resolution

От
Michael Paquier
Дата:
On Wed, May 16, 2018 at 03:37:18PM -0400, Robert Haas wrote:
> On Wed, May 16, 2018 at 3:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > BTW, we seem to have some confusion here already:
> >
> > regression=# create function foo(int) returns int as 'select $1' language sql;
> > CREATE FUNCTION
> > regression=# create procedure foo(text) as 'select $1' language sql;
> > CREATE PROCEDURE
> > regression=# drop function foo;
> > ERROR:  function name "foo" is not unique
> > HINT:  Specify the argument list to select the function unambiguously.
> > regression=# drop routine foo;
> > ERROR:  function name "foo" is not unique
> > HINT:  Specify the argument list to select the function unambiguously.
> > regression=# drop procedure foo;
> > ERROR:  could not find a procedure named "foo"
> >
> > The first two errors are what I'd expect, but why is the third
> > different?
>
> Good question.

I actually find the first error messages ambiguous as well, so that
looks like a bug to me when a lookup is done for those function names.
Shouldn't DROP FUNCTION work only on functions and DROP PROCEDURE only
on procedures?  It is documented that DROP ROUTINE can work on
aggregates, functions and procedures, but the docs tell a different
story about DROP PROCEDURE and DROP FUNCTION.
--
Michael

Вложения

Re: Odd procedure resolution

От
Peter Eisentraut
Дата:
On 5/16/18 15:29, Tom Lane wrote:
> My opinion remains unchanged.  If you're unhappy about the system
> confusing procedure foo(int) and function foo(real), maybe the answer
> is to not overload the name "foo" with such enthusiasm.  But putting
> kluges into (some of) the lookup rules is just going to lead to its
> own problems and surprising results.
> 
> In particular, I dislike the idea that this patch would make routine
> names appear unique in some cases when they do not in others.
> Overloading is complicated/confusing enough without that.

I think I have made a mistake here.  I was reading in between the lines
of a competitor's documentation that they have functions and procedures
in different name spaces, which made me re-read the SQL standard, which
appears to support that approach.

So I'm proposing here a patch to fix that.  It is similar to the patch
proposed earlier in the thread, but more extensive.

One open problem in my patch is that regproc/regprocedure don't have a
way to distinguish functions from procedures.  Maybe a two-argument
version of to_regprocedure?  This will also affect psql's \ef function
and the like.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Вложения

Re: Odd procedure resolution

От
Tom Lane
Дата:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> I think I have made a mistake here.  I was reading in between the lines
> of a competitor's documentation that they have functions and procedures
> in different name spaces, which made me re-read the SQL standard, which
> appears to support that approach.

> So I'm proposing here a patch to fix that.  It is similar to the patch
> proposed earlier in the thread, but more extensive.

> One open problem in my patch is that regproc/regprocedure don't have a
> way to distinguish functions from procedures.  Maybe a two-argument
> version of to_regprocedure?  This will also affect psql's \ef function
> and the like.

TBH, this is several months too late for v11.  You're talking about a
really fundamental redesign, at least if it's done right and not as a
desperate last-minute hack (which is what this looks like).  The points
you make here are just the tip of the iceberg of things that would need
to be reconsidered.

I also remain of the opinion that if we're to separate these namespaces,
the way to do that is to put procedures somewhere other than pg_proc.

Unless you can show that "separate namespaces" is the *only* correct
reading of the SQL spec, which I doubt given the ROUTINE syntax,
I think we're pretty much stuck with the choice we made already.

Or we can push beta back a month or two while we rethink that.
But there's no way you're convincing me that this is a good change
to make four days before beta.

            regards, tom lane


Re: Odd procedure resolution

От
Peter Eisentraut
Дата:
On 5/17/18 16:54, Tom Lane wrote:
> TBH, this is several months too late for v11.

Maybe, but ...

You're talking about a
> really fundamental redesign, at least if it's done right and not as a
> desperate last-minute hack (which is what this looks like).  The points
> you make here are just the tip of the iceberg of things that would need
> to be reconsidered.

I disagree with that assessment.  The patch is large because it reverts
some earlier changes.  But I think the new setup is sound, in large
parts cleaner than before, and addresses various comments that have been
made.  I leave it up to the community to decide whether we want to
address this now or later.

One thing to take into consideration is that leaving things as is for
PG11 and then making this change or one like it in PG12 would create
quite a bit of churn in client programs like psql, pg_dump, and probably
things like pgadmin.

> I also remain of the opinion that if we're to separate these namespaces,
> the way to do that is to put procedures somewhere other than pg_proc.

That would just create an unfathomable amount of code duplication and
mess and unnecessary extra work in PL implementations.

> Unless you can show that "separate namespaces" is the *only* correct
> reading of the SQL spec, which I doubt given the ROUTINE syntax,
> I think we're pretty much stuck with the choice we made already.

I have apparently read the SQL standard differently at different times.
Are you asking me whether I think my current reading is more correct
than my previous ones?  Well, yes.  But someone else should perhaps
check that.  Start with the syntax rules for <SQL-invoked routine>.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: Odd procedure resolution

От
Robert Haas
Дата:
On Thu, May 17, 2018 at 4:10 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> I think I have made a mistake here.  I was reading in between the lines
> of a competitor's documentation that they have functions and procedures
> in different name spaces, which made me re-read the SQL standard, which
> appears to support that approach.

I am really doubtful about trying to merge those completely.  You end
up with confusion about what DROP ROUTINE actually means, for example.
Also, I am quite dubious about the idea that functions, window
functions, and aggregates should go all together into one namespace
and procedures into a completely different one.  I thought merging all
of that stuff down into prokind was quite elegant, and I'm not too
excited about seeing that change backed out.  Functions, procedures,
aggregates, and window functions are all function-like things -- given
any one of them, you might end up writing something like
mything(thingarg1, thingarg2) in some context or other.  I think it is
very sensible to say that we won't let you create two such things with
identical signature, because that's just confusing -- and probably of
very doubtful utility.  At the same time, I don't think that precludes
using context clues to figure out which one must have been intended in
a particular SQL statement.  There are cases where something must
"become all one thing or all the other", but I don't see why that
should be true here.

By the way, if we're going to start worrying about which namespaces
certain competitors put things in, I believe investigation will show
that in at least one notable case, the existing differences are rather
far-reaching and well beyond our ability to fix without major
restructuring of our system catalogs and, I think, abandoning bison.

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


Re: Odd procedure resolution

От
Ashutosh Bapat
Дата:
On Sat, May 19, 2018 at 12:51 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, May 17, 2018 at 4:10 PM, Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
>> I think I have made a mistake here.  I was reading in between the lines
>> of a competitor's documentation that they have functions and procedures
>> in different name spaces, which made me re-read the SQL standard, which
>> appears to support that approach.
>
> I am really doubtful about trying to merge those completely.  You end
> up with confusion about what DROP ROUTINE actually means, for example.
> Also, I am quite dubious about the idea that functions, window
> functions, and aggregates should go all together into one namespace
> and procedures into a completely different one.  I thought merging all
> of that stuff down into prokind was quite elegant, and I'm not too
> excited about seeing that change backed out.  Functions, procedures,
> aggregates, and window functions are all function-like things -- given
> any one of them, you might end up writing something like
> mything(thingarg1, thingarg2) in some context or other.  I think it is
> very sensible to say that we won't let you create two such things with
> identical signature, because that's just confusing -- and probably of
> very doubtful utility.  At the same time, I don't think that precludes
> using context clues to figure out which one must have been intended in
> a particular SQL statement.  There are cases where something must
> "become all one thing or all the other", but I don't see why that
> should be true here.

+1 for all that.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company