Thread: 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