Re: [HACKERS] Implicit casts to text
Awhile back I wrote: Peter Eisentraut [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] Implicit casts to text
Where are we on this? --- Tom Lane wrote: Peter Eisentraut [EMAIL PROTECTED] 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 [EMAIL PROTECTED] http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Implicit casts to text
Bruce Momjian [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] Implicit casts to text
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] 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 [EMAIL PROTECTED] http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Implicit casts to text
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/ ---(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] Implicit casts to text
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 [EMAIL PROTECTED] http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Implicit casts to text
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/ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Implicit casts to text
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/
diff -ur ../cvs-pgsql/src/backend/parser/parse_coerce.c ./src/backend/parser/parse_coerce.c
--- ../cvs-pgsql/src/backend/parser/parse_coerce.c 2007-04-02 08:52:16.0 +0200
+++ ./src/backend/parser/parse_coerce.c 2007-04-03 18:05:12.0 +0200
@@ -371,6 +371,28 @@
r-convertformat = cformat;
return (Node *) r;
}
+ if ((inputTypeId == TEXTOID || targetTypeId == TEXTOID) ccontext == COERCION_EXPLICIT)
+ {
+ /* Explicit coercion through I/O functions */
+ Oid inputTypeOutput;
+ Oid targetTypeInput;
+ bool isVarlena;
+ Oid typeIOParam;
+
+ getTypeInputInfo(targetTypeId, targetTypeInput, typeIOParam);
+ getTypeOutputInfo(inputTypeId, inputTypeOutput, isVarlena);
+
+ result = build_coercion_expression(build_coercion_expression(node, inputTypeOutput, arrayCoerce,
+ CSTRINGOID, -1,
+ cformat,
+ (cformat != COERCE_IMPLICIT_CAST)),
+ targetTypeInput, arrayCoerce,
+ targetTypeId, -1,
+ cformat,
+ (cformat != COERCE_IMPLICIT_CAST));
+
+ return result;
+ }
/* If we get here, caller blew it */
elog(ERROR, failed to find conversion function from %s to %s,
format_type_be(inputTypeId), format_type_be(targetTypeId));
@@ -451,6 +473,10 @@
if (typeInheritsFrom(inputTypeId, targetTypeId))
continue;
+ /* Else only explicit coercion from/to text is possible through I/O functions */
+ if ((inputTypeId == TEXTOID || targetTypeId == TEXTOID) ccontext == COERCION_EXPLICIT)
+ continue;
+
/*
* Else, cannot coerce at this argument position
*/
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [HACKERS] Implicit casts to text
Peter, Which precise implicit casts are we breaking? Can we provide an exact list in the release notes? -- Josh Berkus PostgreSQL @ Sun San Francisco ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] Implicit casts to text
Peter Eisentraut [EMAIL PROTECTED] 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
Re: [HACKERS] Implicit casts to text
Peter Eisentraut [EMAIL PROTECTED] 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
Re: [HACKERS] Implicit casts to text
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/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Implicit casts to text
Peter Eisentraut [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Implicit casts to text
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 [EMAIL PROTECTED] 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 [EMAIL PROTECTED] http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
[HACKERS] 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/ diff -cr ../cvs-pgsql/src/include/catalog/pg_cast.h ./src/include/catalog/pg_cast.h *** ../cvs-pgsql/src/include/catalog/pg_cast.h 2007-02-03 15:06:55.0 +0100 --- ./src/include/catalog/pg_cast.h 2007-04-01 22:26:13.0 +0200 *** *** 264,304 /* * Cross-category casts to and from TEXT - * - * For historical reasons, most casts to TEXT are implicit. This is BAD - * and should be reined in. */ ! DATA(insert ( 20 25 1289 i )); DATA(insert ( 25 20 1290 e )); ! DATA(insert ( 21 25 113 i )); DATA(insert ( 25 21 818 e )); ! DATA(insert ( 23 25 112 i )); DATA(insert ( 25 23 819 e )); ! DATA(insert ( 26 25 114 i )); DATA(insert ( 25 26 817 e )); DATA(insert ( 25 650 1714 e )); ! DATA(insert ( 700 25 841 i )); DATA(insert ( 25 700 839 e )); ! DATA(insert ( 701 25 840 i )); DATA(insert ( 25 701 838 e )); DATA(insert ( 829 25 752 e )); DATA(insert ( 25 829 767 e )); DATA(insert ( 650 25 730 e )); DATA(insert ( 869 25 730 e )); DATA(insert ( 25 869 1713 e )); ! DATA(insert ( 1082 25 749 i )); DATA(insert ( 25 1082 748 e )); ! DATA(insert ( 1083 25 948 i )); DATA(insert ( 25 1083 837 e )); ! DATA(insert ( 1114 25 2034 i )); DATA(insert ( 25 1114 2022 e )); ! DATA(insert ( 1184 25 1192 i )); DATA(insert ( 25 1184 1191 e )); ! DATA(insert ( 1186 25 1193 i )); DATA(insert ( 25 1186 1263 e )); ! DATA(insert ( 1266 25 939 i )); DATA(insert ( 25 1266 938 e )); ! DATA(insert ( 1700 25 1688 i )); DATA(insert ( 25 1700 1686 e )); DATA(insert ( 142 25 2922 e )); DATA(insert ( 25 142 2896 e )); --- 264,301 /* * Cross-category casts to and from TEXT */ ! DATA(insert ( 20 25 1289 a )); DATA(insert ( 25 20 1290 e )); ! DATA(insert ( 21 25 113 a )); DATA(insert ( 25 21 818 e )); ! DATA(insert ( 23 25 112 a )); DATA(insert ( 25 23 819 e )); ! DATA(insert ( 26 25 114 a )); DATA(insert ( 25 26 817 e )); DATA(insert ( 25 650 1714 e )); ! DATA(insert ( 700 25 841 a )); DATA(insert ( 25 700 839 e )); ! DATA(insert ( 701 25 840 a )); DATA(insert ( 25 701 838 e )); DATA(insert ( 829 25 752 e )); DATA(insert ( 25 829 767 e )); DATA(insert ( 650 25 730 e )); DATA(insert ( 869 25 730 e )); DATA(insert ( 25 869 1713 e )); ! DATA(insert ( 1082 25 749 a )); DATA(insert ( 25 1082 748 e )); ! DATA(insert ( 1083 25 948 a )); DATA(insert ( 25 1083 837 e )); ! DATA(insert ( 1114 25 2034 a )); DATA(insert ( 25 1114 2022 e )); ! DATA(insert ( 1184 25 1192 a )); DATA(insert ( 25 1184 1191 e )); ! DATA(insert ( 1186 25 1193 a )); DATA(insert ( 25 1186 1263 e )); ! DATA(insert ( 1266 25 939 a )); DATA(insert ( 25 1266 938 e )); ! DATA(insert ( 1700 25 1688 a )); DATA(insert ( 25 1700 1686 e )); DATA(insert ( 142 25 2922 e )); DATA(insert ( 25 142 2896 e )); *** *** 306,312 /* * Cross-category casts to and from VARCHAR * ! * We support all the same casts as for TEXT, but none are implicit. */ DATA(insert ( 20 1043 1289 a )); DATA(insert ( 1043 20 1290 e )); --- 303,309 /* * Cross-category casts to and from VARCHAR * ! * We support all the same casts as for TEXT. */ DATA(insert ( 20 1043 1289 a )); DATA(insert ( 1043 20 1290 e )); diff -cr ../cvs-pgsql/src/test/regress/expected/foreign_key.out ./src/test/regress/expected/foreign_key.out *** ../cvs-pgsql/src/test/regress/expected/foreign_key.out 2007-02-14 19:35:53.0 +0100 --- ./src/test/regress/expected/foreign_key.out 2007-04-01 22:45:19.0 +0200 *** *** 1125,1134 FOREIGN KEY (x3) REFERENCES pktable(id1); ERROR: foreign key constraint fk_3_1 cannot be implemented DETAIL: Key columns x3 and id1 are of incompatible types: real and integer. ! -- should succeed ! -- int4 promotes to text, so this is allowed (though pretty durn debatable) ALTER TABLE fktable ADD CONSTRAINT fk_1_2 FOREIGN KEY (x1) REFERENCES pktable(id2); -- int4 promotes to real ALTER TABLE fktable ADD CONSTRAINT fk_1_3 FOREIGN KEY (x1) REFERENCES pktable(id3); --- 1125,1136 FOREIGN KEY (x3) REFERENCES pktable(id1); ERROR: foreign key constraint fk_3_1 cannot be implemented DETAIL: Key columns x3 and id1 are of incompatible types: real and integer. ! -- int4 does not promote to text ALTER TABLE fktable ADD CONSTRAINT fk_1_2 FOREIGN KEY (x1) REFERENCES pktable(id2); + ERROR: foreign key constraint fk_1_2 cannot be implemented + DETAIL: Key columns
