Re: Multi-Column List Partitioning - Mailing list pgsql-hackers

Hi Nitin,

Was looking at warnings generated by v8:

partbounds.c:971:17: warning: unused variable 'b1_isnull' [-Wunused-variable]
                                bool        b1_isnull = false;
                                            ^
partbounds.c:972:17: warning: unused variable 'b2_isnull' [-Wunused-variable]
                                bool        b2_isnull = false;

And it seems they've resulted from the above change:

On Mon, Dec 6, 2021 at 10:57 PM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:
> > Looks difficult to understand at first glance, how about the following:
> >
> > if (b1->isnulls != b2->isnulls)
> >    return false;

I don't think having this block is correct, because this says that two
PartitionBoundInfos can't be "logically" equal unless their isnulls
pointers are the same, which is not the case unless they are
physically the same PartitionBoundInfo.  What this means for its only
caller compute_partition_bounds() is that it now always needs to
perform partition_bounds_merge() for a pair of list-partitioned
relations, even if they have exactly the same bounds.

So, I'd suggest removing the block.

> > if (b1->isnulls)
> > {
> >    if (b1->isnulls[i][j] != b2->isnulls[i][j])
> >        return false;
> >    if (b1->isnulls[i][j])
> >        continue;
> > }
> >
> > See how range partitioning infinite values are handled. Also, place
> > this before the comment block that was added for the "!datumIsEqual()"
> > case.
>
> Fixed. I feel the 'continue' block is not required and hence removed it.

Actually, you should've kept the continue block as Amul suggested and
remove the "else" from the following:

                /* < the long comment snipped >*/
                else if (!datumIsEqual(b1->datums[i][j], b2->datums[i][j],
                                  parttypbyval[j], parttyplen[j]))
                    return false;

because with this, list bounds will never be passed to datumIsEqual()
for comparison, even if both are non-NULL.

IOW, the block of code should look as follows, including the comments:

                /*
                 * If the bound datums can be NULL, check that the datums on
                 * both sides are either both NULL or not NULL.
                 */
                if (b1->isnulls)
                {
                    if (b1->isnulls[i][j] != b2->isnulls[i][j])
                        return false;

                    /* Must not pass NULL datums to datumIsEqual(). */
                    if (b1->isnulls[i][j])
                        continue;
                }

                /* < the long comment snipped >*/
                if (!datumIsEqual(b1->datums[i][j], b2->datums[i][j],
                                  parttypbyval[j], parttyplen[j]))
                    return false;

Also, please remove the declarations of b1_isnull and b2_isnull to get
rid of the warnings.


--
Amit Langote
EDB: http://www.enterprisedb.com