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

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

From
Tatsuo Ishii
Date:
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;