public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Richard Guo <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: [email protected]
Subject: Re: Fix NULL pointer reference in _outPathTarget()
Date: Mon, 18 Apr 2022 14:53:41 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAMbWs4-LN=bF8f9eU2R94dJtF54DfDvBq+ovqHnOQqbinYDrUw@mail.gmail.com>
References: <CAMbWs4-LN=bF8f9eU2R94dJtF54DfDvBq+ovqHnOQqbinYDrUw@mail.gmail.com>

Richard Guo <[email protected]> writes:
> The array sortgrouprefs[] inside PathTarget might be NULL if we have not
> identified sort/group columns in this tlist. In that case we would have
> a NULL pointer reference in _outPathTarget() when trying to print
> sortgrouprefs[] with WRITE_INDEX_ARRAY as we are using the length of
> PathTarget->exprs as its array length.

I wondered why we'd not noticed this long since, and the answer is that
it got broken relatively recently by bdeb2c4ec, which removed the former
conditionality of the code:

@@ -2510,14 +2517,7 @@ _outPathTarget(StringInfo str, const PathTarget *node)
     WRITE_NODE_TYPE("PATHTARGET");
 
     WRITE_NODE_FIELD(exprs);
-    if (node->sortgrouprefs)
-    {
-        int            i;
-
-        appendStringInfoString(str, " :sortgrouprefs");
-        for (i = 0; i < list_length(node->exprs); i++)
-            appendStringInfo(str, " %u", node->sortgrouprefs[i]);
-    }
+    WRITE_INDEX_ARRAY(sortgrouprefs, list_length(node->exprs));
     WRITE_FLOAT_FIELD(cost.startup, "%.2f");
     WRITE_FLOAT_FIELD(cost.per_tuple, "%.2f");
     WRITE_INT_FIELD(width);

A semantics-preserving conversion would have looked something like

    if (node->sortgrouprefs)
        WRITE_INDEX_ARRAY(sortgrouprefs, list_length(node->exprs));

I suppose that Peter was trying to remove special cases from the
outfuncs.c code, but do we want to put this one back?  Richard's
proposal would not accurately reflect the contents of the data
structure, so I'm not too thrilled with it.

			regards, tom lane

view thread (6+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Fix NULL pointer reference in _outPathTarget()
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox