Обсуждение: Odd procedure resolution
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
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
Вложения
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
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
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
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
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
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
Вложения
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
Вложения
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
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
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
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