Tom Lane <[email protected]> wrote:

> Fix blatantly broken record_image_cmp() logic for pass-by-value
> fields.
>
> Doesn't anybody here pay attention to compiler warnings?

> http://git.postgresql.org/pg/commitdiff/28858811472f316f73eba0e564837088fc8c6ccd

I don't get a warning on this with either of these compilers,
either with or without asserts enabled:

gcc (Ubuntu/Linaro 4.7.2-2ubuntu1) 4.7.2
Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final) (based on LLVM 3.0)

Bruce had reported a warning, and I was trying to establish whether
a particular change eliminated that warning when the above was
committed.  I really don't like the above "fix", since it only
suppresses the warning without fixing the fundamental problem --
which is that if there is a pass-by-value type with a disallowed
length the comparison would not generate an error in a no-assert
build.  The above patch only changes things from an unpredictable
wrong behavior to a predictable wrong behavior in such cases.

I think something like the attached would make more sense.

Can I get confirmation from someone who can create the warning that
the attached fixes it?

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/utils/adt/rowtypes.c b/src/backend/utils/adt/rowtypes.c
index 33647f7..a0e1a1a 100644
--- a/src/backend/utils/adt/rowtypes.c
+++ b/src/backend/utils/adt/rowtypes.c
@@ -1400,7 +1400,7 @@ record_image_cmp(FunctionCallInfo fcinfo)
 		 */
 		if (!nulls1[i1] || !nulls2[i2])
 		{
-			int			cmpresult = 0;
+			int			cmpresult;
 
 			if (nulls1[i1])
 			{
@@ -1478,7 +1478,12 @@ record_image_cmp(FunctionCallInfo fcinfo)
 						break;
 #endif
 					default:
-						Assert(false);	/* cannot happen */
+						/* cannot happen */
+						elog(ERROR,
+							 "unexpected length of %i found comparing columns of type %s",
+							 (int) tupdesc1->attrs[i1]->attlen,
+							 format_type_be(tupdesc1->attrs[i1]->atttypid));
+						cmpresult = 0;  /* make compiler happy */
 				}
 			}
 			else
@@ -1729,7 +1734,12 @@ record_image_eq(PG_FUNCTION_ARGS)
 						break;
 #endif
 					default:
-						Assert(false);	/* cannot happen */
+						/* cannot happen */
+						elog(ERROR,
+							 "unexpected length of %i found comparing columns of type %s",
+							 (int) tupdesc1->attrs[i1]->attlen,
+							 format_type_be(tupdesc1->attrs[i1]->atttypid));
+						result = false;  /* make compiler happy */
 				}
 			}
 			else
-- 
Sent via pgsql-committers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Reply via email to