vacuum analyze fails: ERROR: Unable to locate type oid 2230924 in catalog

Tatsuo Ishii <t-ishii@sra.co.jp> writes:

*** parse_coerce.c.orig	Sat Feb  3 20:07:53 2001
--- parse_coerce.c	Tue Feb 27 11:33:01 2001
***************
*** 190,195 ****
--- 190,201 ----
Oid			inputTypeId = input_typeids[i];
Oid			targetTypeId = func_typeids[i];

+ 		if (typeidIsValid(inputTypeId) == false)
+ 		    return(false);
+ 
+ 		if (typeidIsValid(targetTypeId) == false)
+ 		    return(false);
+ 
/* no problem if same type */
if (inputTypeId == targetTypeId)
continue;

I'd suggest not arbitrarily erroring out when there is no need for
a conversion, and not doing the cache lookup implied by typeidIsValid
when it's not necessary to touch the type at all. Hence, I'd recommend
moving this down a few lines. Also, conform to the surrounding coding
style and add a comment:

/* don't know what to do for the input type? then quit... */
if (inputTypeId == InvalidOid)
return false;

+		/* don't choke on references to no-longer-existing types */
+ 		if (!typeidIsValid(inputTypeId))
+ 		    return false;
+ 
+ 		if (!typeidIsValid(targetTypeId))
+ 		    return false;

/*
* If input is an untyped string constant, assume we can convert
* it to anything except a class type.
*/

BTW, is this sufficient to prevent the VACUUM failure, or are there more
problems downstream?

regards, tom lane

This looks very safe and I believe should be applied.

vacuum analyze on pg_type fails if bogus entries remain in pg_operator.
Here is a sample script to reproduce the problem.

drop table t1;
create table t1(i int);
drop function foo(t1,t1);
create function foo(t1,t1) returns bool as 'select true' language 'sql';
create operator = (
leftarg = t1,
rightarg = t1,
commutator = =,
procedure = foo
);
drop table t1;
vacuum analyze;

To fix the problem I propose following patches. Comments?
--
Tatsuo Ishii

*** parse_coerce.c.orig	Sat Feb  3 20:07:53 2001
--- parse_coerce.c	Tue Feb 27 11:33:01 2001
***************
*** 190,195 ****
--- 190,201 ----
Oid			inputTypeId = input_typeids[i];
Oid			targetTypeId = func_typeids[i];

+ 		if (typeidIsValid(inputTypeId) == false)
+ 		    return(false);
+ 
+ 		if (typeidIsValid(targetTypeId) == false)
+ 		    return(false);
+ 
/* no problem if same type */
if (inputTypeId == targetTypeId)
continue;

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

I'd suggest not arbitrarily erroring out when there is no need for
a conversion, and not doing the cache lookup implied by typeidIsValid
when it's not necessary to touch the type at all. Hence, I'd recommend
moving this down a few lines. Also, conform to the surrounding coding
style and add a comment:

Thanks for the advice.

/* don't know what to do for the input type? then quit... */
if (inputTypeId == InvalidOid)
return false;

+		/* don't choke on references to no-longer-existing types */
+ 		if (!typeidIsValid(inputTypeId))
+ 		    return false;
+ 
+ 		if (!typeidIsValid(targetTypeId))
+ 		    return false;

I thought "typeidIsValid(targetTypeId) == false" is better than
"!typeidIsValid(targetTypeId)"?

BTW, is this sufficient to prevent the VACUUM failure, or are there more
problems downstream?

The patches fix the particular case. However I'm not sure there is no
lurking problem.
--
Tatsuo Ishii

Tatsuo Ishii <t-ishii@sra.co.jp> writes:

I thought "typeidIsValid(targetTypeId) == false" is better than
"!typeidIsValid(targetTypeId)"?

I've always thought that "== true" and "== false" on something that's
already a boolean are not good style. It's a matter of taste I suppose.
But note that the existing calls on typeidIsValid are coded that way...

regards, tom lane

Tatsuo Ishii <t-ishii@sra.co.jp> writes:

I thought "typeidIsValid(targetTypeId) == false" is better than
"!typeidIsValid(targetTypeId)"?

I've always thought that "== true" and "== false" on something that's
already a boolean are not good style. It's a matter of taste I suppose.
But note that the existing calls on typeidIsValid are coded that way...

regards, tom lane

I see.
--
Tatsuo Ishii

I have comitted the fix to parser/parse_coarse.c. My previous patches
was not correct and I changed the checking right after
typeInheritsFrom.
--
Tatsuo Ishii