Alvaro Herrera <alvherre@commandprompt.com> writes:
> Excerpts from Josh Kupershmidt's message of mié oct 19 23:50:58 -0300 2011:
>> I assume this is an accepted quirk of previous_word() since we have
>> this existing similar code:
> Maybe both are wrong, though the DROP case seems to work so maybe it's
> just dead code.
I looked at the code more closely and I now see what Josh saw and why
this code is like this. If you take a desultory look at previous_word()
you will come away with the impression that it returns NULL when asked
for a word before the first word on the line. But in reality, it
returns NULL only if there is nothing before the current point.
Otherwise, you get duplicates of the first word on the line. For
example in "DROP TABLE <tab>" we'll getprev_wd = TABLEprev2_wd = DROPprev3_wd = DROPprev4_wd = DROPprev5_wd =
DROPprev6_wd= DROP
I think this is just a plain old coding bug: the stop-test assumes that
end is reset to -1 each time through the outer loop, but it isn't.
However, we can't just fix the bug, because if previous_word() actually
starts to return NULLs like its author seems to have intended, the
strcasecmp calls that use its output will dump core.
What I suggest is that we redefine previous_word() as returning an empty
string, not NULL, anytime there is no such preceding word. This is
better than the current behavior because (a) it's less surprising and
(b) it's not ambiguous. Right now the caller can't tell the difference
between "DROP" and "DROP DROP DROP".
With that change, the correct test at line 795 would become
else if (pg_strcasecmp(prev_wd, "DROP") == 0 && prev2_wd[0] == '\0')
(or any other way you care to write a test for empty string). It looks
like there is only one place in the file that is actually expecting a
null result in prev_wd, so there wouldn't be much collateral damage
to fix.
regards, tom lane
