cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
29.97k stars 3.79k forks source link

sql: drop function/procedure with composite/table types doesn't work #114677

Closed DrewKimball closed 10 months ago

DrewKimball commented 10 months ago

If a function or procedure is overloaded with different parameters, with one of those being a composite type, it becomes impossible to drop the func/proc with the composite parameter. Using the name of the type fails, as does surrounding the name in quotes, as well as changing it to uppercase. Surrounding the type name in quotes and changing it to uppercase seems to work, but the func/proc doesn't actually get dropped.

root@localhost:26257/defaultdb> CREATE TYPE t AS (x INT, y INT);
CREATE TYPE

Time: 20ms total (execution 19ms / network 0ms)

root@localhost:26257/defaultdb> CREATE PROCEDURE foo() LANGUAGE SQL AS $$ SELECT 0; $$;
CREATE PROCEDURE

Time: 52ms total (execution 25ms / network 28ms)

root@localhost:26257/defaultdb> CREATE PROCEDURE foo(v t) LANGUAGE SQL AS $$ SELECT 0; $$;
CREATE PROCEDURE

Time: 68ms total (execution 36ms / network 32ms)

root@localhost:26257/defaultdb> \df
List of functions:
  Schema | Name | Result data type | Argument data types | Type
---------+------+------------------+---------------------+-------
  public | foo  | void             |                     | proc
  public | foo  | void             | t                   | proc
(2 rows)
root@localhost:26257/defaultdb> DROP PROCEDURE foo(t);
ERROR: procedure foo(t) does not exist
SQLSTATE: 42883
root@localhost:26257/defaultdb> DROP PROCEDURE foo("t");
ERROR: procedure foo(t) does not exist
SQLSTATE: 42883
root@localhost:26257/defaultdb> DROP PROCEDURE foo(T);
ERROR: procedure foo(t) does not exist
SQLSTATE: 42883
root@localhost:26257/defaultdb> DROP PROCEDURE foo("T");
DROP PROCEDURE

Time: 4ms total (execution 3ms / network 0ms)

root@localhost:26257/defaultdb> \df
List of functions:
  Schema | Name | Result data type | Argument data types | Type
---------+------+------------------+---------------------+-------
  public | foo  | void             |                     | proc
  public | foo  | void             | t                   | proc
(2 rows)

root@localhost:26257/defaultdb> SHOW CREATE PROCEDURE foo;
  procedure_name |          create_statement
-----------------+--------------------------------------
  foo            | CREATE PROCEDURE public.foo()
                 |     LANGUAGE SQL
                 |     AS $$
                 |     SELECT 0;
                 | $$
  foo            | CREATE PROCEDURE public.foo(IN v T)
                 |     LANGUAGE SQL
                 |     AS $$
                 |     SELECT 0;
                 | $$
(2 rows)

Jira issue: CRDB-33621

blathers-crl[bot] commented 10 months ago

Hi @michae2, please add branch-* labels to identify which branch(es) this release-blocker affects.

:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

rafiss commented 10 months ago

I don't think this needs to be a GA blocker, since the same problem affects 23.1:

root@localhost:26257/defaultdb> CREATE TYPE t AS (x INT, y INT);;
CREATE TYPE

Time: 55ms total (execution 49ms / network 6ms)

root@localhost:26257/defaultdb> CREATE FUNCTION foo() returns int  LANGUAGE SQL AS $$ SELECT 0; $$;
CREATE FUNCTION

Time: 65ms total (execution 34ms / network 32ms)

root@localhost:26257/defaultdb> CREATE FUNCTION foo(v t) returns int  LANGUAGE SQL AS $$ SELECT 0; $$;
CREATE FUNCTION

Time: 84ms total (execution 46ms / network 37ms)

root@localhost:26257/defaultdb> \df
List of functions:
  Schema | Name | Result data type | Argument data types | Type
---------+------+------------------+---------------------+-------
  public | foo  | int8             |                     | func
  public | foo  | int8             | t                   | func
(2 rows)
root@localhost:26257/defaultdb> drop function foo(t);
ERROR: function foo(t) does not exist: function undefined
SQLSTATE: 42883
root@localhost:26257/defaultdb> drop function foo("t");
ERROR: function foo(t) does not exist: function undefined
SQLSTATE: 42883
root@localhost:26257/defaultdb> drop function foo(T);
ERROR: function foo(t) does not exist: function undefined
SQLSTATE: 42883
root@localhost:26257/defaultdb> drop function foo("T");
DROP FUNCTION

Time: 6ms total (execution 4ms / network 2ms)

root@localhost:26257/defaultdb> \df
List of functions:
  Schema | Name | Result data type | Argument data types | Type
---------+------+------------------+---------------------+-------
  public | foo  | int8             |                     | func
  public | foo  | int8             | t                   | func
(2 rows)
michae2 commented 10 months ago

I don't think this needs to be a GA blocker, since the same problem affects 23.1

It's a good point, but we are expecting this exact case (UDFs with composite type parameters) to be used much more in 23.2 than 23.1 by a specific customer, so I think we need to figure this out before we announce GA.

rharding6373 commented 10 months ago

The root cause seems to be that we hydrate the t type from DROP PROCEDURE foo(t) as a tuple, but foo(v t)'s overload uses a record type as the parameter type. We determine if the input parameter matches the overload parameter by checking if they are Identical(). An extra wrinkle in that before calling Identical() we turn tuple input parameters into AnyTuple, which I don't think will match even if the overload said it has a tuple type parameter because of the strict comparisons in Identical() compared to Equivalent().

The solution is going to be reconciling this somehow, whether this means hydrating certain composite types as records, using tuple as the overload parameter type for composite types, or treating tuple and record as identical.

rafiss commented 10 months ago

I wonder if this relates to https://github.com/cockroachdb/cockroach/issues/102227

DrewKimball commented 10 months ago

Note: this only reproduces when the function/proc name is overloaded, as in the example.

rharding6373 commented 10 months ago

Addendum to the preceding comment: It does repro when the function is not overloaded, but the workaround is to drop the function/proc without arguments. Here's an example:

root@127.0.0.1:26257/defaultdb> CREATE TYPE t AS (x INT, y INT);                                                      
CREATE TYPE

Time: 12ms total (execution 12ms / network 0ms)

root@127.0.0.1:26257/defaultdb> CREATE PROCEDURE foo(v t) LANGUAGE SQL AS $$ SELECT 0; $$;                            
CREATE PROCEDURE

Time: 29ms total (execution 15ms / network 15ms)

root@127.0.0.1:26257/defaultdb> \df                                                                                   
List of functions:
  Schema | Name | Result data type | Argument data types | Type
---------+------+------------------+---------------------+-------
  public | foo  | void             | t                   | proc
(1 row)
root@127.0.0.1:26257/defaultdb> DROP PROCEDURE foo(t);                                                                
ERROR: procedure foo(t) does not exist
SQLSTATE: 42883
root@127.0.0.1:26257/defaultdb> DROP PROCEDURE foo;                                                                   
DROP PROCEDURE

Time: 131ms total (execution 128ms / network 3ms)

root@127.0.0.1:26257/defaultdb> \df                                                                                   
List of functions:
  Schema | Name | Result data type | Argument data types | Type
---------+------+------------------+---------------------+-------
(0 rows)
DrewKimball commented 10 months ago

An extra wrinkle in that before calling Identical() we turn tuple input parameters into AnyTuple, which I don't think will match even if the overload said it has a tuple type parameter because of the strict comparisons in Identical() compared to Equivalent()

I think this might have been the main problem - it looks like an oversight from when MatchIdentical was written.