Thread: Implicit casts to text
The attached patch changes all implicit casts to text to assignment and cleans up the associated regression test damage. This change has been discussed for the longest time; I propose that we bite the bullet and do it now. The issue described in <http://archives.postgresql.org/pgsql-hackers/2007-02/msg01729.php> should also be fixed but can be considered separately later. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut <peter_e@gmx.net> writes: > The attached patch changes all implicit casts to text to assignment and > cleans up the associated regression test damage. This change has been > discussed for the longest time; I propose that we bite the bullet and > do it now. [ I'm assuming this isn't an April-fool item, otherwise never mind ] The scheme that was in the back of my mind was to do this at the same time as providing a general facility for casting *every* type to and from text, by means of their I/O functions if no specialized cast is provided in pg_cast. This would improve functionality, thus providing a salve to the annoyance of users whose code the restriction breaks: we can certainly argue that it wouldn't do for all those automatically created casts to be implicit. At the same time it'd let us eliminate redundant text-to/from-foo code that's currently in place for some but not all datatypes. If we do only the restrictive part of this, it's a harder sale. So, +1 on the concept, but I think we want a larger patch, and it's probably too late for that for 8.3. regards, tom lane
Am Montag, 2. April 2007 09:17 schrieb Tom Lane: > The scheme that was in the back of my mind was to do this at the same > time as providing a general facility for casting *every* type to and > from text, by means of their I/O functions if no specialized cast is > provided in pg_cast. This would improve functionality, thus providing > a salve to the annoyance of users whose code the restriction breaks: > we can certainly argue that it wouldn't do for all those automatically > created casts to be implicit. At the same time it'd let us eliminate > redundant text-to/from-foo code that's currently in place for some but > not all datatypes. That's the first time I hear of such a scheme. Anyway, the point of this exercise is to reduce misbehavior by explicit casting. I don't see how implicitly adding more casting paths helps that or is even related to that. Even if we had the automatic cast facility that you describe, and I find it highly suspicious, such casts could at most be of the explicit category, so how would that help users who currently rely on the implicit ones? -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut <peter_e@gmx.net> writes: > Am Montag, 2. April 2007 09:17 schrieb Tom Lane: >> The scheme that was in the back of my mind was to do this at the same >> time as providing a general facility for casting *every* type to and >> from text, by means of their I/O functions if no specialized cast is >> provided in pg_cast. > That's the first time I hear of such a scheme. It's been discussed before, eg http://archives.postgresql.org/pgsql-admin/2004-06/msg00390.php http://archives.postgresql.org/pgsql-hackers/2004-10/msg00303.php > Anyway, the point of this > exercise is to reduce misbehavior by explicit casting. I don't see how > implicitly adding more casting paths helps that or is even related to that. > Even if we had the automatic cast facility that you describe, and I find it > highly suspicious, such casts could at most be of the explicit category, so > how would that help users who currently rely on the implicit ones? Certainly they'd all be explicit-only. From a technical perspective there's no need to do the two things at the same time; I'm just opining that we could sell it easier if we did them together. If we just do this part, what users will see is that we broke their queries for what to them will appear to be no particular gain. regards, tom lane
Added to TODO: * Allow all data types to cast to and from TEXT http://archives.postgresql.org/pgsql-hackers/2007-04/msg00017.php --------------------------------------------------------------------------- Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > The attached patch changes all implicit casts to text to assignment and > > cleans up the associated regression test damage. This change has been > > discussed for the longest time; I propose that we bite the bullet and > > do it now. > > [ I'm assuming this isn't an April-fool item, otherwise never mind ] > > The scheme that was in the back of my mind was to do this at the same > time as providing a general facility for casting *every* type to and > from text, by means of their I/O functions if no specialized cast is > provided in pg_cast. This would improve functionality, thus providing > a salve to the annoyance of users whose code the restriction breaks: > we can certainly argue that it wouldn't do for all those automatically > created casts to be implicit. At the same time it'd let us eliminate > redundant text-to/from-foo code that's currently in place for some but > not all datatypes. > > If we do only the restrictive part of this, it's a harder sale. > > So, +1 on the concept, but I think we want a larger patch, and it's > probably too late for that for 8.3. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 5: don't forget to increase your free space map settings -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Am Montag, 2. April 2007 18:41 schrieb Tom Lane: > Certainly they'd all be explicit-only. From a technical perspective > there's no need to do the two things at the same time; I'm just opining > that we could sell it easier if we did them together. If we just do > this part, what users will see is that we broke their queries for what > to them will appear to be no particular gain. I find this method of selling features very unusual. The two issues under consideration have nothing in common except that they have "cast" in their subject line. The reduction of implicit casts to text has to stand on its own: the purpose is to produce more reliable expression behavior. Those whose queries this would break are not helped by having other casts available without work; they'd still have to do manual fixups. So what we'd have is "Sorry, casting int to text implicitly doesn't work anymore, but instead you can cast $othertype to text explicitly." How does that help anyone? -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut wrote: > Am Montag, 2. April 2007 18:41 schrieb Tom Lane: > > Certainly they'd all be explicit-only. ?From a technical perspective > > there's no need to do the two things at the same time; I'm just opining > > that we could sell it easier if we did them together. ?If we just do > > this part, what users will see is that we broke their queries for what > > to them will appear to be no particular gain. > > I find this method of selling features very unusual. The two issues under > consideration have nothing in common except that they have "cast" in their > subject line. The reduction of implicit casts to text has to stand on its > own: the purpose is to produce more reliable expression behavior. Those > whose queries this would break are not helped by having other casts available > without work; they'd still have to do manual fixups. So what we'd have > is "Sorry, casting int to text implicitly doesn't work anymore, but instead > you can cast $othertype to text explicitly." How does that help anyone? I assumed the issue was that there might not be explicit casts for every case were were now disallowing. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Am Dienstag, 3. April 2007 17:17 schrieb Bruce Momjian: > I assumed the issue was that there might not be explicit casts for every > case were were now disallowing. My proposal is to "downgrade" some casts from implicit to assignment. Tom's proposal is to add more casts at the level of explicit, which is farther below assignment. No cast will be lost. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Am Montag, 2. April 2007 18:41 schrieb Tom Lane: > >> The scheme that was in the back of my mind was to do this at the same > >> time as providing a general facility for casting *every* type to and > >> from text, by means of their I/O functions if no specialized cast is > >> provided in pg_cast. > http://archives.postgresql.org/pgsql-admin/2004-06/msg00390.php > http://archives.postgresql.org/pgsql-hackers/2004-10/msg00303.php FWIW, is the attached patch about what you had in mind? (It probably only covers "normal" types at the moment.) -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter, Which precise implicit casts are we breaking? Can we provide an exact list in the release notes? -- Josh Berkus PostgreSQL @ Sun San Francisco
Peter Eisentraut <peter_e@gmx.net> writes: > FWIW, is the attached patch about what you had in mind? (It probably only > covers "normal" types at the moment.) Hm, I hadn't realized that it would take as little work as that ... I have an itchy feeling that you missed something but I'm not sure what. One thing I had wanted to do is take out the existing functions and pg_cast entries that are effectively just providing hard-wired equivalents to this, but that's merely housekeeping. regards, tom lane
Where are we on this? --------------------------------------------------------------------------- Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > FWIW, is the attached patch about what you had in mind? (It probably only > > covers "normal" types at the moment.) > > Hm, I hadn't realized that it would take as little work as that ... > I have an itchy feeling that you missed something but I'm not sure > what. > > One thing I had wanted to do is take out the existing functions and > pg_cast entries that are effectively just providing hard-wired > equivalents to this, but that's merely housekeeping. > > regards, tom lane > > ---------------------------(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 -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes: > Where are we on this? Since there weren't any objections, I guess we can do it ;-) I'll try to do something with Peter's patch plus removing the deadwood. Would you add his patch to the queue so I don't forget? regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Where are we on this? > > Since there weren't any objections, I guess we can do it ;-) > > I'll try to do something with Peter's patch plus removing the deadwood. > Would you add his patch to the queue so I don't forget? Added. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Awhile back I wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> FWIW, is the attached patch about what you had in mind? (It probably only >> covers "normal" types at the moment.) > Hm, I hadn't realized that it would take as little work as that ... > I have an itchy feeling that you missed something but I'm not sure > what. On closer inspection, my gut feeling was right: this patch doesn't work, because it assumes that I/O functions have the same calling conventions as cast functions, which they don't --- the semantics for second and third arguments, if used, are different. We could tweak build_coercion_expression() to generate the correct arguments, but there are more problems downstream, eg in exprIsLengthCoercion() which will misinterpret the arguments in such a node. So I'm inclined to think that we really need a specialized expression node type, perhaps "CoerceViaIO". This might have some usefulness in plpgsql too, if it could piggyback on that rather than doing its own ad-hoc conversions. An alternative, which would be less pretty but also less work, is to implement the cast this way only for types with single-argument input functions. Those that require extra args would still have to have explicit pg_cast entries. The main problem with this is we can't support varchar on the same basis as text, because its input function wants extra arguments. BTW, I note that uuid has snuck in with assignment casts to and from text. Is this really what we want if we're tightening up everything else? regards, tom lane