Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[YSQL] The "alter function/procedure depends on extension" variant causes a spurious warning but produces the expected result #11523

Open
bllewell opened this issue Feb 18, 2022 · 2 comments
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue

Comments

@bllewell
Copy link
Contributor

bllewell commented Feb 18, 2022

Jira Link: DB-804

The tests that are described in this report and that are implemented in the attached issue-11523.zip were run after starting my YB cluster with this YSQL configuration parameter setting:

ysql_suppress_unsupported_error=true

The default for this parameter is false. And when the default setting is used, every 0A000 occurrence, that in my tests is a warning and that I suppress by setting client_min_messages to error, is a genuine error—meaning that any statement that causes 0A000 rolls back and has no effect.

Tested in YB-2.13.0.1.

Here is the basic setup. It runs without error in both PG and YB. Do it first in PG:

\c postgres postgres
set client_min_messages = warning;
drop database if exists db;
create database db owner postgres;

\c db postgres
\set VERBOSITY verbose
set client_min_messages = warning;
drop schema if exists public cascade;
create schema s authorization postgres;

prepare qry as
select
  p.proname::text,
  p.pronamespace::regnamespace::text,
  e.extname
from
  pg_catalog.pg_proc p
  inner join
  pg_catalog.pg_depend d
  on p.oid = d.objid
  inner join
  pg_catalog.pg_extension e
  on d.refobjid = e.oid
where p.pronamespace::regnamespace::text = 's'
order by 1, 2, 3;

create extension tablefunc with schema s;

create function s.f()
  returns text
  language plpgsql
as $body$
begin
  return 'f';
end;
$body$;

create procedure s.p()
  language plpgsql
as $body$
begin
  assert true;
end;
$body$;

execute qry;

Here is the "execute qry" result:

   proname   | pronamespace |  extname  
-------------+--------------+-----------
 connectby   | s            | tablefunc
 connectby   | s            | tablefunc
 connectby   | s            | tablefunc
 connectby   | s            | tablefunc
 crosstab    | s            | tablefunc
 crosstab    | s            | tablefunc
 crosstab    | s            | tablefunc
 crosstab2   | s            | tablefunc
 crosstab3   | s            | tablefunc
 crosstab4   | s            | tablefunc
 normal_rand | s            | tablefunc

The listed functions are all brought directly by installing tablefunc. So far, s.f() and s.p() aren't listed.

Next, demo the "alter function/procedure... depends on extension..." feature like this:

alter function s.f() depends on extension tablefunc;
alter procedure s.p() depends on extension tablefunc;

Execute f() and p() to show that they (still) behave as expected:

do $body$
declare
  v text;
begin
  assert (select s.f()) = 'f';
end;
$body$;

call s.p();

Both finish silently as expected. Now repeat this:

execute qry;

This is the new result:

   proname   | pronamespace |  extname  
-------------+--------------+-----------
 connectby   | s            | tablefunc
 connectby   | s            | tablefunc
 connectby   | s            | tablefunc
 connectby   | s            | tablefunc
 crosstab    | s            | tablefunc
 crosstab    | s            | tablefunc
 crosstab    | s            | tablefunc
 crosstab2   | s            | tablefunc
 crosstab3   | s            | tablefunc
 crosstab4   | s            | tablefunc
 f           | s            | tablefunc
 normal_rand | s            | tablefunc
 p           | s            | tablefunc

Notice that s.f() and s.p() have been added to the list, as expected.

Now drop the extension and re-execute the dictionary query:

drop extension tablefunc restrict;
execute qry;

The "drop extension" completes silently—even though this removes the dependent function s.f() and the dependent procedure s.p() . This silent removal, even with "restrict", might seem strange. But it is the intended behavior.

Notice that now "execute qry" gets no rows.

Confirm that s.f() and s.p() have been cascade-dropped like this:

do $body$
declare
  v text;
begin
  assert (select s.f()) = 'f';
  assert false, 'Should not be here';
exception
  when undefined_function then
    assert true, 'Caught "undefined_function"';
end;
$body$;

do $body$
begin
  call s.p();
  assert false, 'Should not be here';
exception
  when undefined_function then
    assert true, 'Caught "undefined_function"';
end;
$body$;

Each test finishes silently, as expected.

Clean up like this:

\c postgres postgres
set client_min_messages = warning;
drop database db;

See the attached issue-11523.zip. It contains demo.sql, pg-demo.txt, and yb-demo.txt. Notice \o pg-demo.txt at the start of demo.sql. Execute the script in PG to produce the PG spool file. It completes silently.

Now edit the script to start with \o yb-demo.txt and execute it in YB-2.13.0.1. Now each of the "alter function\procedure" attempts causes these warnings on the terminal screen.

WARNING:  0A000: This statement not supported yet
HINT:  Please report the issue on https://github.com/YugaByte/yugabyte-db/issues

But the script finishes without error.

Diff yb-demo.txt and pg-demo.txt. They are identical. This shows that the "alter function\procedure" functionality (seems to) work as expected. But there are spurious errors left over from an earlier YB version where the functionality wasn't yet implemented.

@bllewell
Copy link
Contributor Author

issue-11523.zip

@bllewell
Copy link
Contributor Author

bllewell commented Apr 17, 2022

See also Issue # 2717. This was opened when no variant of "alter function" or "alter procedure was supported. Now (17-Apr-2022 using YB-2.13.0.1) all variants of "alter function" except for "depends on extension" work without error or warning. But every variant of "alter function" except for "depends on extension" works without error but causes this (differently worded) spurious warning:

ALTER PROCEDURE not supported yet

Please fix both issues, #2717 and #11523, in the same pull request.

@bllewell bllewell changed the title [YSQL] Neither "alter function" nor "alter procedure" supports the "depends on extension" variant [YSQL] The "alter function/procedure depends on extension" variant causes a spurious warning but produces the expected result Apr 18, 2022
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue
Projects
None yet
Development

No branches or pull requests

2 participants