Thread: vacuum analyze fails: ERROR: Unable to locate type oid 2230924 in catalog
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;
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
Re: vacuum analyze fails: ERROR: Unable to locate type oid 2230924 in catalog
From
Bruce Momjian
Date:
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, Pennsylvania19026
Re: vacuum analyze fails: ERROR: Unable to locate type oid 2230924 in catalog
From
Tatsuo Ishii
Date:
> 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
Re: vacuum analyze fails: ERROR: Unable to locate type oid 2230924 in catalog
From
Tatsuo Ishii
Date:
> 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
Re: vacuum analyze fails: ERROR: Unable to locate type oid 2230924 in catalog
From
Tatsuo Ishii
Date:
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 > 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;