yugabyte / yugabyte-db

YugabyteDB - the cloud native distributed SQL database for mission-critical applications.
https://www.yugabyte.com
Other
8.9k stars 1.06k forks source link

spurious 'function "hello" already exists with same argument types' error after drop #1956

Open bllewell opened 5 years ago

bllewell commented 5 years ago

Jira Link: DB-1345 Until what I describe here is fixed, it is—IMHO—impossible to develop a practical system of stored procedures because, sooner or later, the repeated use of drop procedure and create procedure brought by iterative improvement of the code will trigger the problem.

Note: I know about create or replace. But you need the explicit drop when you change the parameter list because doing this makes a new overload and would, without drop, clutter the schema with a gazillion superannuated overloads. Moreover, during iterative development, you might well return to an earlier overload design after first abandoning it. The ensuing collision will cause problems unless you're very tidy with the "garbage collection".

I believe that the root of the problem is that, in YB Version 1.3.0, DDLs have non-negotiable autocommit behavior while in the vanilla PostgreSQL, they do not. Here's a simple demo to show this confusing autocommit behavior. Notice that I do all SQLs after an explicit begin command to insulate myself from the True or False value of the AUTOCOMMIT mode of the ysqlsh session. First, ensure first that table t doesn't already exist

begin;
drop table if exists t;
commit;

This quietly succeeds and so is already confusing because if the drop autocommitted, then commit should draw the "there is no transaction in progress" warning. But it does not. Now create the table, but roll back the creation:

begin;
create table t(v varchar(20));
rollback;

This, too, quietly succeeds—so the same confusion again. Now try to insert into table t:

begin;
insert into t(v) values('table t exists');
select v from t;
rollback;

This shows that table t does exist and so proves that create table autocommits.

Now the stage is set for the real bug.

This atypically written code succeeds quietly on each successive repetition after the first (where the drop causes "NOTICE: procedure hello() does not exist, skipping" as expected):

-- The AUTOCOMMIT mode should have no effect on the following
-- because of the explicit use of "begin".
\set AUTOCOMMIT False

begin;
drop procedure if exists hello();
commit;

begin;
create procedure hello()
language plpgsql as $$
begin
  raise notice 'Hello';
end
$$;
commit;

begin;
call hello();
rollback;

Here is a reproducible way to provoke problems:

\set AUTOCOMMIT False
drop procedure if exists hello();
create procedure hello()
  language plpgsql as $$
begin
  raise notice 'Hello';
end
$$;

This causes "ERROR: function "hello" already exists with same argument types". You must issue rollback after this failure before trying anything else. With this precaution, the first code section above succeeds again.

However, it's easy to get into a mess. While I was developing this testcase, all sorts of errors like I show next occurred. I (These examples are copied-and-pasted from the ysqlsh terminal window.)

=  create procedure hello()
-  language plpgsql as $$
$  begin
$    raise notice 'Hello';
$  end
$  $$;
=  commit;
=  
=  begin;
=  call hello();
ERROR:  procedure hello() does not exist
LINE 1: call hello();

The create procedure quietly succeeded. But the call claimed that it doesn't exist. This can be fixed by exiting the ysqlsh session and restarting it:

> begin;
> call hello();
ERROR:  procedure hello() does not exist
LINE 1: call hello();
             ^
HINT:  No procedure matches the given name and argument types. You might need to add explicit type casts.
> rollback;
> ^D
$ ysql -h 127.0.0.1 -p 5433 -d postgres -U postgres
> 
> begin;
> call hello();
NOTICE:  Hello
> rollback;

Now the call worked OK. I haven't been able to tie down the steps to reproduce this.

srhickma commented 5 years ago

@bllewell regarding your example using create table t(v varchar(20)); followed by rollback;, the table is persisting after the rollback because DDLs are not currently transactional (see #1404), so rolling back will not remove tables, functions, procedures, etc. created inside the transaction, regardless of the AUTOCOMMIT setting.

As for the main issue, the fact that disconnecting and reconnecting fixed the issue (in the case of the procedure not existing) makes me think it might be a caching problem, but I will look into this further.

bllewell commented 5 years ago

Here's a little more info. Make sure that the table accounts exists. Then repeat this indefinitely. It always succeeds:

\set AUTOCOMMIT False
begin;
drop table if exists accounts;
create table accounts(
  id int generated by default as identity,
  name varchar(10) not null,
  balance dec(15,2) not null,
  constraint accounts_pk primary key(id)
);
commit;

Now make sure that the function value_of_balance(in the_name varchar) exists (and that you can execute it without error). Check it with this:

\df value_of_balance

It shows that this:

 Schema |       Name       | Result data type |    Argument data types     | Type 
--------+------------------+------------------+----------------------------+------
 public | value_of_balance | numeric          | the_name character varying | func

Then repeat this indefinitely. It always fails:

\set AUTOCOMMIT False
begin;
drop function if exists value_of_balance(in the_name varchar);
create function value_of_balance(in the_name varchar)
  returns dec
  language plpgsql
as '
  declare
    the_balance dec(15,2);
  begin
    select a.balance
    into the_balance
    from accounts a
    where a.name = the_name;

    return the_balance;
  end;
';
commit;

The drop always goes ahead quietly. And the create always fails with this:

ERROR:  function "value_of_balance" already exists with same argument types

Now do this:

\df value_of_balance

It shows that there's nothing there.

I do appreciate that there's a workaround. But it would take real users a long time to find it. And there's no mental model that explains why the workaround is needed and works. Notice the conspicuous asymmetry between dropping and creating a table and dropping and creating a function.

srhickma commented 5 years ago

@bllewell I tried to reproduce the above error with the latest master, but I am not seeing any errors. Here is the script I am running:

create table accounts(
  id int generated by default as identity,
  name varchar(10) not null,
  balance dec(15,2) not null,
  constraint accounts_pk primary key(id)
);

\set AUTOCOMMIT False
begin;
drop table if exists accounts;
create table accounts(
  id int generated by default as identity,
  name varchar(10) not null,
  balance dec(15,2) not null,
  constraint accounts_pk primary key(id)
);
commit;

create function value_of_balance(in the_name varchar)
  returns dec
  language plpgsql
as '
  declare
    the_balance dec(15,2);
  begin
    select a.balance
    into the_balance
    from accounts a
    where a.name = the_name;

    return the_balance;
  end;
';

\set AUTOCOMMIT False
begin;
drop function if exists value_of_balance(in the_name varchar);
create function value_of_balance(in the_name varchar)
  returns dec
  language plpgsql
as '
  declare
    the_balance dec(15,2);
  begin
    select a.balance
    into the_balance
    from accounts a
    where a.name = the_name;

    return the_balance;
  end;
';
commit;

I am also seeing the correct output from \df value_of_balance. Let me know if I have misinterpreted your example.

bllewell commented 5 years ago

It sounds like the problem that I observed in YB version 1.3.0 has vanished in the later in-development version that you tested. That's good news.