Обсуждение: 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
От
Bruce Momjian
Дата:
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
> 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 > 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;