[YSQL] "alter procedure" causes spurious warning

[bllewell]

Jira Link: DB-1669
I need to use this syntax:

alter procedure <my schema>.<my proc>(...)
set search_path = <my schema>;

In a multi-user, multi-schema application design in pursuit of good security. I want to lock down name resolution to prevent subversion of my design intention by nefariously created synonyms and the like.

Attempting this in YB Version 2.0.1 causes this error:

ERROR:  ALTER PROCEDURE not supported yet
LINE 1: alter procedure code.insert_master_and_details(
        ^
HINT:  See https://github.com/YugaByte/yugabyte-db/issues/1155. Click '+' on the description to raise its priority

Notice that Issue #1155 is closed, and is too generic for the present use case. See my new issue #2718 asking that the HINT text be updated to point to this issue (#2717).

See Issue #17315. This shows that if you restart the YB cluster with this YSQL configuration parameter setting:

ysql_suppress_unsupported_error=true

then every 0A000 error becomes a warning and that, not withstanding what the message says, metadata queries show that the alter statement (in at least some variants) has its intended effect. Specific tests for this variant (not shown here) show that it, too, does, then, indeed have its intended effect.

[tylarb]

Hi!

I was able to compile oracle comparability functions for postgres (orafce - https://github.com/orafce/orafce) using postgres extension library - just a "make" with gcc, g++, bin including pg_config and LD_LIBRARY_PATH including postgres/lib.
However, once I opened DB, I cannot create orafce extension due to this issue.

yugabyte=# CREATE EXTENSION orafce;
ERROR:  ALTER FUNCTION not supported yet
HINT:  See https://github.com/YugaByte/yugabyte-db/issues/2717. Click '+' on the description to raise its priority

Thanks

[ramsrivatsa]

@bllewell -- ALTER FUNCTION works. There is still support needed for alter procedure/routine/aggregate.

[bllewell]

This has been open for over two years. I re-tested in YB-2.11.1.0. An alter procedure attempt still causes the same "0A000: ALTER PROCEDURE not supported yet" with the same reference to this issue.

Notice that create or replace (or drop and then create) is a viable workaround for alter. But this does require that you have the source definition to hand. If you drop it, you also have to re-grant any privileges that have been granted on it.

[bllewell]

Summary

alter function works now (in YB-2.13.0.1) —and you don't get any warnings.

alter procedure also works but draws warnings.

@m-iancu, @ramsrivatsa: Pls. change the code to remove these spurious warnings. (But see Issue #11523.)

Detail

Try the testcase below first in vanilla PG and then in YB. Make sure to change the name of the spool file to reflect the environment so that you can use your favorite diff program to compare the two files. I used PG Version 14.1 and YB-2.13.0.1.

The script exercises every single variant of alter procedure according to the account in the PostgreSQL doc, with the exception of alter procedure depends on extension. Notice that both of "alter function/procedure depends on extension" work bu case a spurius error. Issue #11523 tracks this.

(Notice that Issue #11495 points out that neither alter procedure nor alter function is yet documented in the YSQL doc.)

In YB, every other alter procedure execution draws a warning except for this one, which completes silently:

alter procedure s1.p(text) owner to joe_blow;

This one:

alter procedure s1.p(text) set schema s2;

draws this warning:

ALTER PROCEDURE SET SCHEMA not supported yet

And every other alter procedure variant draws this warning:

ALTER PROCEDURE not supported yet

In PG (of course) the script finishes silently. But the two spool files are identical.

Note: warnings go to stdout and not to the spool file. This allows the the spooled output to be identical in YB and PG despite the warnings that are drawn in YB.

TESTCASE

\o pg-spool.txt
\c postgres postgres
set client_min_messages = warning;
drop database if exists db;
create database db owner postgres;

\c db postgres
set client_min_messages = warning;
drop schema if exists public cascade;
drop user if exists joe_blow;
create user joe_blow login password 'p';
create schema pg authorization postgres;
create schema s1 authorization postgres;
create schema s2 authorization postgres;

drop view if exists pg.procedures cascade;
create view pg.procedures(
  owner,
  schema,
  name,
  security,
  proconfig)
as
select
  proowner::regrole::text,
  pronamespace::regnamespace::text,
  proname::text,
  case
    when prosecdef then 'definer'
    else 'invoker'
  end,
  proconfig
from pg_catalog.pg_proc
where
  pronamespace::regnamespace::text in ('s1','s2') and
  pronargs = 1
  and prokind = 'p';

prepare qry as
select * from pg.procedures limit 1;

create procedure s1.p(msg in text)
  language plpgsql
  security definer
  set datestyle = 'PostgreSQL, YMD'
as $body$
begin
  assert false, msg;
end;
$body$;
execute qry;
\t on

alter procedure s1.p(text) owner to joe_blow;
execute qry;

alter procedure s1.p(text) set schema s2;
execute qry;

alter procedure s2.p(text) rename to q;
execute qry;

alter procedure s2.q(text) security invoker;
execute qry;

alter procedure s2.q(text) set timezone = 'Europe/London';
execute qry;

set timezone = 'America/New_York';
alter procedure s2.q(text) set timezone from current;
execute qry;

alter procedure s2.q(text) reset timezone;
execute qry;

alter procedure s2.q(text) set timezone = 'Asia/Kathmandu';
execute qry;

alter procedure s2.q(text) reset all;
execute qry;
--------------------------------------------------------------------------------
-- Clean up
\c postgres postgres
set client_min_messages = warning;
drop database if exists db;
drop user joe_blow

\t off
\o

Here is the spooled output:

  owner   | schema | name | security |           proconfig           
----------+--------+------+----------+-------------------------------
 postgres | s1     | p    | definer  | {"DateStyle=PostgreSQL, YMD"}

 joe_blow | s1     | p    | definer  | {"DateStyle=PostgreSQL, YMD"}

 joe_blow | s2     | p    | definer  | {"DateStyle=PostgreSQL, YMD"}

 joe_blow | s2     | q    | definer  | {"DateStyle=PostgreSQL, YMD"}

 joe_blow | s2     | q    | invoker  | {"DateStyle=PostgreSQL, YMD"}

 joe_blow | s2     | q    | invoker  | {"DateStyle=PostgreSQL, YMD",TimeZone=Europe/London}

 joe_blow | s2     | q    | invoker  | {"DateStyle=PostgreSQL, YMD",TimeZone=America/New_York}

 joe_blow | s2     | q    | invoker  | {"DateStyle=PostgreSQL, YMD"}

 joe_blow | s2     | q    | invoker  | {"DateStyle=PostgreSQL, YMD",TimeZone=Asia/Kathmandu}

 joe_blow | s2     | q    | invoker  | 

See the attached issue-2717.zip. It contains demo.sql, pg-spool.txt, and yb-spool.txt. Execute demo.sql first in PG (completes silently) and then in YB (causes spurious warnings). Make sure to set the \o spool file name appropriately for PG or YB before running. Diff the spool files. They are identical.

[bllewell]

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