Обсуждение: BUG #6212: PREPARE(pseudotype) should be blocked off

The following bug has been logged online:

Bug reference:      6212
Logged by:          Caleb Welton
Email address:      Caleb.Welton@emc.com
PostgreSQL version: 9.0
Operating system:   OSX
Description:        PREPARE(pseudotype) should be blocked off
Details:

statements such as:
  PREPARE p1(anyelement) AS SELECT quote_literal($1);
  PREPARE p2(internal) AS SELECT int2recv($1);

Should not be allowed.

They can result in peculiar behavior such as the following:

execute p1('hello')
ERROR:  cannot display a value of type anyelement

"Caleb Welton" <Caleb.Welton@emc.com> writes:
> statements such as:
>   PREPARE p1(anyelement) AS SELECT quote_literal($1);
>   PREPARE p2(internal) AS SELECT int2recv($1);
> Should not be allowed.

Hmm.  It would require an extra catalog lookup per parameter to enforce
that.  Not sure that it's worth it just to prevent "peculiar" errors.
Can you point to any worse consequences?

            regards, tom lane

On Sep 16, 2011, at 11:11 AM, Tom Lane wrote:

> "Caleb Welton" <Caleb.Welton@emc.com> writes:
>> statements such as:
>>  PREPARE p1(anyelement) AS SELECT quote_literal($1);
>>  PREPARE p2(internal) AS SELECT int2recv($1);
>> Should not be allowed.
>=20
> Hmm.  It would require an extra catalog lookup per parameter to enforce
> that.  Not sure that it's worth it just to prevent "peculiar" errors.
> Can you point to any worse consequences?
>=20
>             regards, tom lane


I haven't found any more severe issues and I'll agree its not a high priori=
ty item.  But the fix is simple enough that I don't see a reason to ignore =
it either.

The easiest fix would be, as you say, adding one extra syscache lookup:

static Query *
transformPrepareStmt(ParseState *pstate, PrepareStmt *stmt)
{
...
        foreach(l, stmt->argtypes)
        {
            TypeName   *tn =3D lfirst(l);
            Oid            toid =3D typenameTypeId(pstate, tn);

    >        /* Pseudotypes are not valid parameters to PREPARE */
    >        if (get_typtype(toid) =3D=3D TYPTYPE_PSEUDO)
    >        {
    >            ereport(ERROR,
    >                    (errcode(ERRCODE_INDETERMINATE_DATATYPE),
    >                     errmsg("type \"%s\" is not a valid parameter for PREPARE",
    >                            TypeNameToString(tn))));
    >        }

            argtoids[i++] =3D toid;
        }
...
}


If you really don't like the extra syscache lookup I'd offer two alternativ=
e implementations:

1) Creating a new macro IsPseudoType() like the existing IsPolymorphicType(=
) macro given that we already have the oid.
2) TypenameGetTypid already has the whole cache tuple which contains both t=
he pieces of information we want, but it only returns the oid... easy enoug=
h to fix, but larger api impact.

Regards,
  Caleb

<Caleb.Welton@emc.com> writes:
> On Sep 16, 2011, at 11:11 AM, Tom Lane wrote:
>> Hmm.  It would require an extra catalog lookup per parameter to enforce
>> that.  Not sure that it's worth it just to prevent "peculiar" errors.
>> Can you point to any worse consequences?

> I haven't found any more severe issues and I'll agree its not a high priority item.  But the fix is simple enough
thatI don't see a reason to ignore it either. 

> The easiest fix would be, as you say, adding one extra syscache lookup:

If it were just PREPARE I'd have done it without quibbling; that isn't
something I regard as a critical performance path.  But if we're trying
to lock this out then we logically have to enforce the same restriction
in exec_parse_message, and that *is* a performance-critical path.  Plus
it has no existing catalog lookup that might be kluged to pass back the
extra information.

Maybe we should do it anyway, but I'd really like to see a more
significant reason.

            regards, tom lane