cockroachdb / cockroachdb/cockroach
restore: OnFailOrCancel leaves a dangling function reference when restoring a function into an existing schema
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
When a `RESTORE` brings in a function whose parent schema **already exists** in the target cluster, `createImportingDescriptors` registers the new function's signature in the existing schema's `Functions` map ([restore_job.go#L1936](https://github.com/cockroachdb/cockroach/blob/4809d0bbc652c4ad6db067af3778ed55a58d8c3a/pkg/backup/restore_job.go#L1936)).
If that restore then fails or is canceled, `dropDescriptors` (the `OnFailOrCancel` path) drops the function descriptor it created ([restore_job.go#L3716](https://github.com/cockroachdb/cockroach/blob/4809d0bbc652c4ad6db067af3778ed55a58d8c3a/pkg/backup/restore_job.go#L3716)) but never removes that signature from the pre-existing schema. The schema is `ToExisting`, so cleanup never touches it. The result is a schema descriptor that references a function descriptor which no longer exists — a dangling reference that fails descriptor validation.
There is an analogous cleanup for the type/table case (`removeExistingTypeBackReferences`, [restore_job.go#L3951](https://github.com/cockroachdb/cockroach/blob/4809d0bbc652c4ad6db067af3778ed55a58d8c3a/pkg/backup/restore_job.go#L3951)), which strips back-references installed on pre-existing types. There is no equivalent for functions added to pre-existing schemas.
**To Reproduce**
Start a single-node cluster (`cockroach start-single-node --insecure` or `cockroach demo --insecure`), then:
```sql
-- Source: a function in a schema, and a table that depends on it.
CREATE DATABASE db;
USE db;
CREATE SCHEMA sc;
CREATE FUNCTION sc.fn() RETURNS INT LANGUAGE SQL AS $$ SELECT 42 $$;
CREATE TABLE sc.t (id INT PRIMARY KEY, derived INT DEFAULT sc.fn());
INSERT INTO sc.t (id) VALUES (1), (2);
BACKUP DATABASE db INTO 'nodelocal://1/fnbug';
-- Recreate db + schema WITHOUT the function, so the restore must bring the
-- function back as a fresh descriptor into a pre-existing schema (sc).
DROP DATABASE db CASCADE;
CREATE DATABASE db;
USE db;
CREATE SCHEMA sc;
-- Pause the restore after descriptors are created but before publish.
SET CLUSTER SETTING jobs.debug.pausepoints = 'restore.before_flow';
RESTORE TABLE db.sc.* FROM LATEST IN 'nodelocal://1/fnbug' WITH detached;
```
Record the job ID returned by the `RESTORE`. Once it is paused, cancel it:
```sql
SET CLUSTER SETTING jobs.debug.pausepoints = '';
CANCEL JOB ;
```
After the job reaches `canceled`, inspect descriptor validity:
```sql
SELECT database_name, schema_name, error FROM crdb_internal.invalid_objects;
```
This reports the dangling reference:
```
db | sc | invalid function 1XX in schema "sc" (1YY)
```
The `restore.before_flow` pausepoint is only a deterministic way to hit the window; any restore that fails after `createImportingDescriptors` but before publish (e.g. a real error during data flow) goes through the same `OnFailOrCancel` path and reproduces it.
As a control, repeating the steps but `RESUME`-ing the job instead of canceling completes successfully and leaves `crdb_internal.invalid_objects` empty — so the defect is in the cleanup path.
**Expected behavior**
On cleanup, the function signature added to the pre-existing schema should be removed alongside the dropped function descriptor, leaving no invalid objects — matching the success path.
Contributor guide
Assessment
This issue has not been assessed yet.