Tom Lane wrote:
Joe Conway <[EMAIL PROTECTED]> writes:
Well, maybe the attached patches better explain what I mean.
In the case of the 8.2 patch, a very small code change allows new
regression data including NULL rowids to:
1) not crash
2) have no impact otherwise
The much bigger 8.3 patch shows that for the very same new regression
data, there is a significant impact on the output (i.e. NULL rowids get
their own output row as discussed).
I'm still leaning toward applying the 8.2 patch for back branches but
I'll bow to the general consensus.
I'd vote for the bigger patch all the way back. The smaller patch has
nothing to recommend it except being smaller. It replaces the crash
with a behavior that will change in 8.3, thus creating a potential
portability issue for users of (post-repair) back branches. Why not
get it right the first time?
OK, I can live with that.
A couple of minor thoughts:
* You could reduce the ugliness of many of the tests by introducing a
variant strcmp function that does the "right" things with NULL inputs.
It might also be worth adding a variant pstrdup that takes a NULL.
I had thoughts along those lines -- it would certainly make the code
more readable. I'll go ahead and do that but it won't be in time for a
26 October beta2.
* Surely this bit:
xpfree(lastrowid);
! if (rowid)
! lastrowid = pstrdup(rowid);
}
needs to be:
if (rowid)
lastrowid = pstrdup(rowid);
else
lastrowid = NULL;
no? (Again the variant pstrdup would save some notation)
Well I had already defined xpfree like this:
8<------------------
#define xpfree(var_) \
do { \
if (var_ != NULL) \
{ \
pfree(var_); \
var_ = NULL; \
} \
} while (0)
8<------------------
so lastrowid is already NULL (I sometimes wish this was the default
behavior for pfree() itself). But the point about pstrdup variant is
well taken, and I guess the xpfree behavior is not obvious, so it
deserves at least a comment.
regards, tom lane
PS: I hear things are pretty crazy out your way -- hope the fire's
not too close to you.
We packed and were ready to evacuate two or three times, but never
actually had to leave our house, thankfully. The closest the fire ever
got was about 4 miles, and at this point I don't think we're in any more
direct danger. But I know many people who were not so fortunate :-(
Joe
---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
