Re: [HACKERS] quote_literal(integer) does not exist
Tom Lane wrote: I don't offhand see anything else I'd consider weakening the casting rules for. If anyone else is interested, I took ... substring(text,integer) | substring(text,integer,integer) | substring(text,text) | substring(text,text,text) | texticlike(text,text) | ~~* texticnlike(text,text)| !~~* texticregexeq(text,text) | ~* texticregexne(text,text) | !~* textlike(text,text) | ~~ textnlike(text,text) | !~~ textregexeq(text,text)| ~ textregexne(text,text)| !~ upper(text) | Thoughts? In one of our applications, we have some numbers (e.g. product numbers) that have meaning attached to individual digits. Product numbers usually contain letters, too, but for historical reasons they do not in this application. So we put them into integer columns for efficiency. We still want to run queries like product_no LIKE '51%' on them. Well, for the application, I don't see much of a problem here. This will probably cost 3-5 lines of code in the whole application. It will just cause some inconvenience when working with psql interactively. And I have not yet seen another DBMS that does not accept almost any input type for the typical string operations such as substring or LIKE. It feels a little bit strange that I will have to do all that typecasting now. Just my $0.02. Best Regards Michael Paesold ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] quote_literal(integer) does not exist
On Sat, 24 Nov 2007 21:17:39 -0500 Tom Lane wrote: Andreas 'ads' Scherbaum [EMAIL PROTECTED] writes: we have some plpgsql functions which use quote_literal() regardless of the data type. With Beta 3 this does not work anymore[1]. If you're unwilling to fix your application, you can hack around that for yourself. regression=# select quote_literal(42); ERROR: function quote_literal(integer) does not exist LINE 1: select quote_literal(42); ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts. regression=# create function quote_literal(anyelement) returns text as $$ regression$# select pg_catalog.quote_literal($1 :: pg_catalog.text) regression$# $$ language sql; CREATE FUNCTION regression=# select quote_literal(42); quote_literal --- '42' (1 row) Already had a similar function in my test case, but yours is more elegant. I also think, that we will fix our applications or at least most of them. But that's not the point: more people will run into this problem and this looks like a showstopper for updating to 8.3. By the way, the function is named quote_literal(), not quote_text(). From my point of view i expect to get everything correctly quoted, what's feeded as input into this function. Given the fact, that previous versions accepted every input without notice about the implicit cast, i don't see not so much blame in the application. Kind regards -- Andreas 'ads' Scherbaum PostgreSQL User Group Germany ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] quote_literal(integer) does not exist
On Nov 25, 2007 11:51 PM, Andreas 'ads' Scherbaum [EMAIL PROTECTED] wrote: But that's not the point: more people will run into this problem and this looks like a showstopper for updating to 8.3. By the way, the function is named quote_literal(), not quote_text(). From my point of view i expect to get everything correctly quoted, what's feeded as input into this function. Given the fact, that previous versions accepted every input without notice about the implicit cast, i don't see not so much blame in the application. I had a similar experience to Andreas when I first migrated my app to 8.3b1 for testing. It wasn't hard to fix, but did seem like an unnecessary barrier to upgrading. I agree that the name of the function (and its behavior up till now) encourage users to think of it as a quote whatever I throw at you function, which is indeed what you would want/expect in the context of building a dynamic query. Having quote_literal take its argument as text was fine whilst we were automatically casting arguments, but in 8.3 it seems that quote_literal(anyelement) makes a lot more sense than quote_literal(text). So, I wonder why we don't just adapt the internal function to take anyelement? It would save a lot of apps from being broken by the move to 8.3, and make the function more convenient. Regards, BJ ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] quote_literal(integer) does not exist
Hello Having quote_literal take its argument as text was fine whilst we were automatically casting arguments, but in 8.3 it seems that quote_literal(anyelement) makes a lot more sense than quote_literal(text). So, I wonder why we don't just adapt the internal function to take anyelement? It would save a lot of apps from being broken by the move to 8.3, and make the function more convenient. It's good idea. I'll look on it. It needs change of pgproc :( Pavel ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] quote_literal(integer) does not exist
Hello I sent patch to pg_patches. Regards Pavel Stehule ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] quote_literal(integer) does not exist
Brendan Jurd [EMAIL PROTECTED] 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. 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. 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. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] quote_literal(integer) does not exist
On 25/11/2007, Tom Lane [EMAIL PROTECTED] wrote:
Brendan Jurd [EMAIL PROTECTED] 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 know so is too late now. This patch can be saved for 8.4.
There is one reason for patching. It is consistence with || operator.
That's all. This patch doesn't change casting rules, and these rules
are strict still (what is good). But quote_literal is more natural
with anyelement parameter, than only text element. There isn't any
reason for some limit. This discus is not about change rules, but
about change of definition of some functions.
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.
Reason is same. Consistence with || operator. But, equivalent in SQL is:
CREATE OR REPLACE FUNCTION quote_ident(anyelement)
RETURNS text AS
SELECT CASE when $1::text LIKE '% %' - first problem
THEN '' || $1 || ''
ELSE $1::text END - second problem
$$ LANGUAGE SQL;
so, I see, with quote_ident I was wrong. I belive so with anyelement
can be more usable, but it is different than quote_literal.
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.
probably quote_literal is more important than others. It is wide used
with dynamic selects. On other side, any similar problem can be simple
solved with custom wrapper (with some note in release notes).
I found more important problem. I cannot simply use literal in
polymorphic function. I cannot call anyfce('literal') what is
acceptable in SQL or plpgsql languages, but not in C language. Sure,
this topic is for 8.4.
nice a day
Pavel Stehule
regards, tom lane
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
Re: [HACKERS] quote_literal(integer) does not exist
On Nov 26, 2007 5:23 AM, Tom Lane [EMAIL PROTECTED] wrote: Brendan Jurd [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] quote_literal(integer) does not exist
On Mon, 26 Nov 2007 06:35:20 +1100 Brendan Jurd wrote: On Nov 26, 2007 5:23 AM, Tom Lane [EMAIL PROTECTED] wrote: 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. The problem for me is: we expect and encourage people to do safe programming and now they have to debug their programs and remove some of the safe parts just to make PostgreSQL happy. As you said, that is not, what the average programmer expect. 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. Yes, Tom Lane is right that the current behavior is broken. But the solution cannot be to exclude anything beside text but instead we should move forward to accept anything (at least, if it's possible). 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. True. You can't even create a table who's name is just an integer or where the name starts with an integer, so in any way you already have to use quotes and you are aware of the problem. 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. Yeah, that's what a beta is for. We don't expect to have people running production systems with beta software so it needs an reinstall anyway after the release. Kind regards -- Andreas 'ads' Scherbaum PostgreSQL User Group Germany ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] quote_literal(integer) does not exist
Brendan Jurd [EMAIL PROTECTED] writes:
On Nov 26, 2007 5:23 AM, Tom Lane [EMAIL PROTECTED] wrote:
... 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 did the same search. Ignoring cases where it's fairly obvious that
you're trying to apply a textual operation to something non-textual
(eg, LIKE and btrim(), though both of these have been complained of
since beta started...), it seems that quote_literal() has a good case,
and you could also make an argument for allowing a non-text second
argument for set_config(), since these things used to work:
regression=# select set_config('work_mem', 1000, false);
set_config
1000kB
(1 row)
regression=# select set_config('random_page_cost', 2.5, false);
set_config
2.5
(1 row)
I don't find that amazingly compelling, but it's not silly either.
If we were to do this I think I'd introduce set_config(text,float8,bool)
rather than going all the way with anyelement, though. That would be
enough to cover both the int and float cases, as well as anyone who
likes to spell booleans as 1 and 0.
I don't offhand see anything else I'd consider weakening the casting
rules for. If anyone else is interested, I took
select p.oid::regprocedure as regprocedure, oprname from pg_proc p left
join pg_operator o on (oprcode = p.oid) where 25 = any (proargtypes)
and removed duplicates and obviously-internal functions such as textout,
leaving me with this list:
regprocedure| oprname
---+-
array_to_string(anyarray,text)|
btrim(text) |
btrim(text,text) |
character_length(text)|
convert_to(text,name) |
initcap(text) |
length(text) |
lower(text) |
lpad(text,integer)|
lpad(text,integer,text) |
ltrim(text) |
ltrim(text,text) |
md5(text) |
octet_length(text)|
overlay(text,text,integer)|
overlay(text,text,integer,integer)|
position(text,text) |
quote_ident(text) |
quote_literal(text) |
regexp_matches(text,text) |
regexp_matches(text,text,text)|
regexp_replace(text,text,text)|
regexp_replace(text,text,text,text) |
regexp_split_to_array(text,text) |
regexp_split_to_array(text,text,text) |
regexp_split_to_table(text,text) |
regexp_split_to_table(text,text,text) |
repeat(text,integer) |
replace(text,text,text) |
rpad(text,integer)|
rpad(text,integer,text) |
rtrim(text) |
rtrim(text,text) |
set_config(text,text,boolean) |
split_part(text,text,integer) |
string_to_array(text,text)|
strpos(text,text) |
substr(text,integer) |
substr(text,integer,integer) |
substring(text,integer) |
substring(text,integer,integer) |
substring(text,text) |
substring(text,text,text) |
texticlike(text,text) | ~~*
texticnlike(text,text)| !~~*
texticregexeq(text,text) | ~*
texticregexne(text,text) | !~*
textlike(text,text) | ~~
textnlike(text,text)
Re: [HACKERS] quote_literal(integer) does not exist
Tom Lane [EMAIL PROTECTED] writes: Thoughts? I wouldn't have thought set_config was frequently enough used to warrant a catalog change but quote_literal does seem like a common enough use case to warrant it and once we're doing it there... I started to think md5(bytea) would be reasonable but then I checked and we do in fact already have that. Perhaps it would be interesting to have some of the other functions like overlay or replace for bytea as well in the future but I hardly think anyone is going to complain urgently given that using them via free casts to text would never have really worked that well previously. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] quote_literal(integer) does not exist
Tom Lane wrote: [...] it seems that quote_literal() has a good case, and you could also make an argument for allowing a non-text second argument for set_config() [...] Thoughts? I think there is just enough of a case for quote_literal(), although in my experience the vast majority of places where it is used in fact should be replaced by using placeholders. I find that I need to use it very rarely indeed. The case for set_config() is less compelling. cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[HACKERS] quote_literal(integer) does not exist
Hello all, testing 8.3b3, i found out an interesting thing: we have some plpgsql functions which use quote_literal() regardless of the data type. With Beta 3 this does not work anymore[1]. Given the fact, that some functions do a lot of work, you (or at least we) don't want to look, if the data you just moving around is from type integer, text or something else. So in the past we just quoted everything which worked fine. I can understand, that enforcing a strict type checking is a fine thing. But given the fact, that PG did a lot of implicit typecasting in the past, removing this is not a real world solution. This will surely prevent some more people from upgrading to 8.3 because the previous fine-working applications will stop working on 8.3. A quote_literal() which can cope with any data type, maybe combined with a warning, would be a better way for a smooth upgrade. Kind regards 1: http://archives.postgresql.org/pgsql-hackers/2007-08/msg00697.php -- Andreas 'ads' Scherbaum PostgreSQL User Group Germany ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] quote_literal(integer) does not exist
Andreas 'ads' Scherbaum [EMAIL PROTECTED] writes: we have some plpgsql functions which use quote_literal() regardless of the data type. With Beta 3 this does not work anymore[1]. If you're unwilling to fix your application, you can hack around that for yourself. regression=# select quote_literal(42); ERROR: function quote_literal(integer) does not exist LINE 1: select quote_literal(42); ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts. regression=# create function quote_literal(anyelement) returns text as $$ regression$# select pg_catalog.quote_literal($1 :: pg_catalog.text) regression$# $$ language sql; CREATE FUNCTION regression=# select quote_literal(42); quote_literal --- '42' (1 row) regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
