Sean Chittenden <chitt@speakeasy.net> writes:
>> Can you add some regression tests, please?
> Given the simplicity of the casts, does this really need a
> require a regression test?
That request seems quite over-the-top to me too. The real problem here
is just whether we want to be accepting a feature addition, small though
it be, at this stage of the beta cycle. I've got mixed emotions about
that myself. The odds of breaking anything with this patch seem
essentially zero, and we have certainly seen this requested multiple
times before, so one part of me says "sure, push it in". But from a
project-management standpoint it's hard to justify not saying "it's got
to wait for 8.1".
>> The patch treats any non-zero value as "true". Is that the behavior we
>> want, or should we only allow "1" as an integer representation of
>> "true"? (I'm not sure myself, I just don't think copying C here is
>> necessarily the best guide.)
> I would posit that this is the desired behavior as it's consistent with
> every language I can think of.
I can't see anything wrong with it either. Throwing an error for
anything except 0 or 1 would be the other possibility, but that doesn't
seem like it really buys anything.
>> The patch changes the system catalog; it probably ought to also bump
>> the catalog version number.
> System catalog bumps have been coming through with some degree of
> regularity so I wasn't worried about providing the patch to bump the
> catalog date. -sc
I think the agreed protocol is that the descriptive text should
remind the committer to bump the catversion upon application. Just
to make sure he doesn't forget ;-)
regards, tom lane
