Thread: Re: [PATCHES] Small fix for _equalValue()
> > 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:
...
> 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.
OK, I've committed a patch to the stable and development trees which
adds a guard check in three places in gram.y, matching the guards
already in place for other cass in the same area.
Fernando, can you please check this (preferably without your patches to
guard the output functions, since those mask the upstream problem)?
Or, can you give me the complete test case you are using to demonstrate
the problem to make sure I'm testing the thing you are seeing?
While I'm looking at it, the "SET key=val" area is somewhat "kludge on
kludge" to enable lists of values, at least partly because it was at the
end of the development cycle and I didn't want to ripple breakage into
parts of the code I wasn't trying to touch. I'll go through and try to
rationalize it sometime soon (actually, I've already started but haven't
finished).
- Thomas
Thomas Lockhart <thomas@fourpalms.org> writes:
> Fernando, can you please check this (preferably without your patches to
> guard the output functions, since those mask the upstream problem)?
> Or, can you give me the complete test case you are using to demonstrate
> the problem to make sure I'm testing the thing you are seeing?
I believe the test case is just to compile tcop/postgres.c with
COPY_PARSE_PLAN_TREES #defined, and see if the regression tests pass...
regards, tom lane
> I believe the test case is just to compile tcop/postgres.c with
> COPY_PARSE_PLAN_TREES #defined, and see if the regression tests pass...
OK, that works, afaik without having or requiring Fernando's patches.
I'll plan on going through the parser code a bit more in the next week
or so.
- Thomas
Thomas Lockhart <thomas@fourpalms.org> writes:
>> I believe the test case is just to compile tcop/postgres.c with
>> COPY_PARSE_PLAN_TREES #defined, and see if the regression tests pass...
> OK, that works, afaik without having or requiring Fernando's patches.
That's because I already committed the other changes he pointed out ;-).
But yeah, we seem to be copy-clean again.
regards, tom lane
...
> That's because I already committed the other changes he pointed out ;-).
> But yeah, we seem to be copy-clean again.
I had thought that you objected to the guard code in the copy functions
since nodes should not have had the content they did. And afaik I have
now fixed the upstream problems with the content.
Had you changed you mind about the necessity for the guard code? Why did
those patches get applied if the only feedback in the thread was that
the problem did not lie there?
Or are we talking about two different parts of the patch submission? I'm
a bit confused as to the current state of the code tree...
- Thomas
Thomas Lockhart <thomas@fourpalms.org> writes:
>> That's because I already committed the other changes he pointed out ;-).
>> But yeah, we seem to be copy-clean again.
> I had thought that you objected to the guard code in the copy functions
> since nodes should not have had the content they did. And afaik I have
> now fixed the upstream problems with the content.
Right, the SET DEFAULT problem is fixed that way. Fernando had pointed
out a couple of problems in unrelated constructs (GRANT and something
else I forget now) that also needed to be fixed. Those fixes did get
committed.
> Had you changed you mind about the necessity for the guard code?
No. I think Value is fine as-is.
regards, tom lane
> No. I think Value is fine as-is.
Ah, great.
- Thomas