builtins: pg_get_constraintdef broken during DROP CONSTRAINT in txn

[otan]

This is a new failure introduced between 4d6d79d...7897f24, so my guess is 617d300 is the cause of the regression.

The following should not error:

BEGIN;
create table tbl (price int, quantity int);
alter table tbl add constraint quan_check check (quantity > 0);
alter table tbl add constraint pr_check check (price > 0);

SELECT conname, pg_get_constraintdef(c.oid) AS constraintdef, c.convalidated AS valid
            FROM pg_constraint c
            JOIN pg_class t ON c.conrelid = t.oid
            WHERE c.contype = 'c'
              AND t.relname = 'tbl'; -- this query beforehand  is required to trigger the error; without it everything is fine
alter table tbl drop constraint quan_check;
SELECT conname, pg_get_constraintdef(c.oid) AS constraintdef, c.convalidated AS valid
            FROM pg_constraint c
            JOIN pg_class t ON c.conrelid = t.oid
            WHERE c.contype = 'c'
              AND t.relname = 'tbl'; -- this fails
COMMIT;

example output:

[email protected]:26257/defaultdb> BEGIN;
create table tbl (price int, quantity int);
alter table tbl add constraint quan_check check (quantity > 0);
alter table tbl add constraint pr_check check (price > 0);

SELECT conname, pg_get_constraintdef(c.oid) AS constraintdef, c.convalidated AS valid
            FROM pg_constraint c
            JOIN pg_class t ON c.conrelid = t.oid
            WHERE c.contype = 'c'
              AND t.relname = 'tbl'; 
alter table tbl drop constraint quan_check;
SELECT conname, pg_get_constraintdef(c.oid) AS constraintdef, c.convalidated AS valid
            FROM pg_constraint c
            JOIN pg_class t ON c.conrelid = t.oid
            WHERE c.contype = 'c'
              AND t.relname = 'tbl';
COMMIT;
BEGIN


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

CREATE TABLE


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

ALTER TABLE


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

ALTER TABLE


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

   conname   |     constraintdef      | valid
-------------+------------------------+--------
  pr_check   | CHECK ((price > 0))    | true
  quan_check | CHECK ((quantity > 0)) | true
(2 rows)


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

ALTER TABLE


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

ERROR: pg_get_constraintdef(): unknown constraint (OID=1220880198)
SQLSTATE: 22023
ROLLBACK


Time: 2ms total (execution 1ms / network 2ms)

pg_get_constraintdef:

cockroach/pkg/sql/sem/builtins/pg_builtins.go

Lines 300 to 319 in be1b6c4

	func makePGGetConstraintDef(argTypes tree.ArgTypes) tree.Overload {
	return tree.Overload{
	Types: argTypes,
	ReturnType: tree.FixedReturnType(types.String),
	Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) {
	r, err := ctx.Planner.QueryRowEx(
	ctx.Ctx(), "pg_get_constraintdef",
	ctx.Txn,
	sessiondata.NoSessionDataOverride,
	"SELECT condef FROM pg_catalog.pg_constraint WHERE oid=$1", args[0])
	if err != nil {
	return nil, err
	}
	if len(r) == 0 {
	return nil, pgerror.Newf(pgcode.InvalidParameterValue, "unknown constraint (OID=%s)", args[0])
	}
	return r[0], nil
	},
	Info: notUsableInfo,
	Volatility: tree.VolatilityStable,

Refs #68363 (comment)

[blathers-crl]

Hi @otan, please add a C-ategory label to your issue. Check out the label system docs.

While you're here, please consider adding an A- label to help keep our repository tidy.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[ajwerner]

In #69008 we changed where we decide to reset the complete set of descriptors. I'm not totally sure I picked up on that subtle detail. We use to do it when we did a write. The idea was that's when we'd invalidate the set. @postamar changed it to be on read, which I sort of get, but I don't totally get.

cockroach/pkg/sql/catalog/descs/descriptor.go

Line 237 in be1b6c4

tc.kv.releaseAllDescriptors()

The idea being that you've got to check things out before you check them in, but I don't know, it doesn't seem to work here. I will note that new descriptors we avoid the whole descs collection. I don't know why that is.

[postamar]

@fqazi I understand you've got a fix in the works, related to the random schema changer somehow. Feel free to re-triage this if this issue assignment doesn't make sense.

[fqazi]

I'll split it off and put up a pull request for it, I have it with the randomized schema changer fixes.