Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings - Mailing list pgsql-hackers

After further study I've understood what was bothering me about
the logic in compareJsonbContainers (lines 280ff).  Because
JsonbIteratorNext doesn't presently guarantee to set val->type
when returning WJB_DONE, the stanza appears to be at risk of
undefined behavior should one iterator return that while the other
returns something else.  The comment fails to discuss this case,
and the code doesn't consider it either.  But in reality, the case
cannot happen for the exact same reason that WJB_END_ARRAY and
WJB_END_OBJECT can't occur: our earlier tests of object and array
structure & length matching guarantee that we can't reach the end
of one input before the other.

So I think we need to rewrite that comment and extend the assertions,
along the lines of the separate patch attached.  This gets through
check-world, which is unsurprising because if it did not we'd have
seen use-of-undefined-value Valgrind complaints long since.

I still think that we should silence the compiler warnings by
removing the undefined-ness per my previous patch.  I'm inclined
to back-patch that (for people compiling old branches with latest
compiler) but apply this bit to HEAD only (since it's just a code
clarification).

            regards, tom lane

diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index c8b6c15e059..e73906dc09d 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -277,22 +277,16 @@ compareJsonbContainers(JsonbContainer *a, JsonbContainer *b)
         else
         {
             /*
-             * It's safe to assume that the types differed, and that the va
-             * and vb values passed were set.
-             *
-             * If the two values were of the same container type, then there'd
-             * have been a chance to observe the variation in the number of
-             * elements/pairs (when processing WJB_BEGIN_OBJECT, say). They're
-             * either two heterogeneously-typed containers, or a container and
-             * some scalar type.
-             *
-             * We don't have to consider the WJB_END_ARRAY and WJB_END_OBJECT
-             * cases here, because we would have seen the corresponding
-             * WJB_BEGIN_ARRAY and WJB_BEGIN_OBJECT tokens first, and
-             * concluded that they don't match.
+             * It's not possible for one iterator to report end of array or
+             * object while the other one reports something else, because we
+             * would have detected a length mismatch when we processed the
+             * container-start tokens above.  Likewise we can't see WJB_DONE
+             * from one but not the other.  They're either two different-type
+             * containers, or a container and some scalar type, or two
+             * different scalar types.  Sort on the basis of the type.
              */
-            Assert(ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
-            Assert(rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);
+            Assert(ra != WJB_DONE && ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
+            Assert(rb != WJB_DONE && rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);

             Assert(va.type != vb.type);
             Assert(va.type != jbvBinary);