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.84k stars 3.77k forks source link

sql,backupccl: RESTORE with PLPGSQL function fails #116480

Closed stevendanna closed 8 months ago

stevendanna commented 8 months ago

Describe the problem

On master (830dad6b1da950bc47e05c1116c7f7c61994bbca), we cannot seem to restore a database with a PLPGSQL function. I'm still investigating the root cause, but to reproduce:

CREATE SEQUENCE seq;
CREATE OR REPLACE FUNCTION foo() RETURNS INT AS
$$
  BEGIN
    RETURN nextval('seq');
  END
$$ LANGUAGE PLPGSQL
backup database defaultdb into 'userfile:///backup';
restore database defaultdb from latest in 'userfile:///backup' with new_db_name=new_defaultdb;

I get:

ERROR: at or near "return": syntax error
SQLSTATE: 42601
DETAIL: source SQL:
BEGIN
RETURN nextval('seq':::STRING)
^
HINT: try \h BEGIN

I haven't yet investigated further, but am marking as a GA-blocker until I can.

Jira issue: CRDB-34638

blathers-crl[bot] commented 8 months ago

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

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

blathers-crl[bot] commented 8 months ago

cc @cockroachdb/disaster-recovery

stevendanna commented 8 months ago

I started looking into this because I was curious about whether the name->ID rewriting here https://github.com/cockroachdb/cockroach/pull/115809 required additional restore support since we rewrite descriptor IDs. However, I don' think this PR can be the cause of this failure because it hasn't merged to 23.2 yet but I can reproduce the above on beta3.

stevendanna commented 8 months ago

Some printf debugging says the error is coming from:

https://github.com/cockroachdb/cockroach/blob/830dad6b1da950bc47e05c1116c7f7c61994bbca/pkg/sql/catalog/rewrite/rewrite.go#L916-L919

stevendanna commented 8 months ago

I think the problem is that the rewrite code for functions all call parser.Parse(). I'm not familiar with the APIs here, but looking at other code that deals with functions, it looks like we likely want a switch that also knows how to call plpgsql.Parser: https://github.com/cockroachdb/cockroach/blob/830dad6b1da950bc47e05c1116c7f7c61994bbca/pkg/sql/catalog/rewrite/rewrite.go#L327