Re: quote_literal(integer) does not exist

On Nov 26, 2007 5:23 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Brendan Jurd" <direvus(at)gmail(dot)com> writes:
> > So, I wonder why we don't just adapt the internal function to take
> > anyelement?
>
> The main argument against is the "slippery slope" one: once we accept
> this, what else? The entire point of that change was to make people
> be aware of where they are inducing text coercions, and deciding to
> hide that again on the basis of individual complaints is no way to
> design a system.

I'm all for the idea of making people conscious of text coercions in
general, but in the *particular* case of quote_literal, having it only
accept text is undesirable, unintuitive and most importantly, it will
break apps which otherwise may have been able to enjoy a smooth
transition to 8.3.

I would argue that quote_literal should have been set up to accept
anyelement in the very first place, and I'd guess that the original
choice of text as an argument type was partially driven by the
understanding that everything gets coerced to text, making it a de
facto "anyelement" substitute. Or maybe anyelement wasn't available
when it was introduced. Either way, if quote_literal() is all about
safely stuffing variables into dynamic queries, the new behaviour is a
regression. In context, it makes perfect sense to throw integers,
numerics and whatever else at quote_literal and expect it to Just
Work.

My feeling is that the change in text coercion behaviour has well
illuminated that the text argument type for quote_literal isn't ideal.
Great! Let's fix it.

> As a not-too-far-away example, I see that the proposed patch Pavel
> sent in arbitrarily decides to change quote_ident() too, which was
> not asked for and has got much less justification than changing
> quote_literal(). That sort of cowboy approach to semantics is not
> the way to proceed.

I'd pass on changing quote_ident. It seems natural for it to take a
text argument. I can imagine a lot of people using, say,
quote_literal(int) in the field; I can't imagine the same for
quote_ident.

> Another issue is that changing pg_proc.h without forcing initdb
> is not good practice. We are far enough along in the beta cycle
> that we shouldn't force initdb lightly, and I definitely *don't*
> want to do it again next week when someone else comes up with
> some other "must have" auto-coercing function. If anyone wants
> to make a serious argument for this, look through the whole of
> pg_proc.h, see what else needs to be changed at the same time,
> and make a coherent proposal.

I took your suggestion and looked through all the procs that take a
text argument. I honestly didn't see anything else I thought needed
to change.

So my proposal is to add your quote_literal(anyelement) SQL function
to pg_proc and be done with it.

I can see your reluctance to force an initdb, but what's the greater
mischief; forcing initdb in beta, or breaking applications on release?
My personal perspective is that it's an easy choice ... avoid
breaking the apps, that's what betas are for.

Thanks for your time,
BJ