Re: Small fix for _equalValue()

Tom Lane wrote:
>
> Fernando Nasser <fnasser(at)redhat(dot)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.
>

I see. Thanks, I will fix the patch and resubmit it (see below).

> 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.
>

Someone is using NULL strings in gram.y, like in:

VariableSetStmt: SET ColId TO var_value
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n->name = $2;
n->args = makeList1(makeStringConst($4, NULL));
$$ = (Node *) n;
}

there are several instances of it, all related to variable set.

Well, NULL is a valid value for a (char *) so this seems legal
enough to me.

I still think we should handle NULL pointer values in equalValue.
(we can through an ERROR if we decide to disallow NULL pointers in
Value -- we must go after who added it to VariableSet or revert that
change though).

> 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?
>

Yes, but I will double check all the same.

Thanks for the comments.

--
Fernando Nasser
Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9