Re: Small fix for _equalValue() - Mailing list pgsql-patches

From Tom Lane
Subject Re: Small fix for _equalValue()
Date
Msg-id 9965.1015514079@sss.pgh.pa.us
Whole thread Raw
In response to Small fix for _equalValue()  (Fernando Nasser <fnasser@redhat.com>)
List pgsql-patches
Fernando Nasser <fnasser@redhat.com> writes:
> *************** _equalValue(Value *a, Value *b)
> *** 1771,1777 ****
>           case T_Float:
>           case T_String:
>           case T_BitString:
> !             return strcmp(a->val.str, b->val.str) == 0;
>           default:
>               break;
>       }
> --- 1771,1780 ----
>           case T_Float:
>           case T_String:
>           case T_BitString:
> !             if ((a->val.str != NULL) && (b->val.str != NULL))
> !                 return strcmp(a->val.str, b->val.str) == 0;
> !             else
> !                 return a->val.ival == b->val.ival;  /* true if both are NULL */
>           default:
>               break;
>       }

Several comments here:

This is not the idiomatic way to do it; there is an equalstr() macro
in equalfuncs.c that does this pushup for you.  So "return
equalstr(a->val.str, b->val.str)" would be the appropriate fix.

Possibly a more interesting question, though, is *why* equalValue is
seeing Values with null pointer parts.  I cannot think of any good
reason to consider that a legal data structure.  Do you know where this
construct is coming from?  I'd be inclined to consider the source at
fault, not equalValue.

On the other fixes: as a rule, a field-typing bug in copyfuncs suggests
an equivalent bug over in equalfuncs, and vice versa; as well as
possible errors in readfuncs/outfuncs.  Did you look?

            regards, tom lane

pgsql-patches by date:

Previous
From: Tom Lane
Date:
Subject: Re: Small fix for _equalValue()
Next
From: Fernando Nasser
Date:
Subject: Re: Small fix for _equalValue() REPOST