Re: Strange behavior with leap dates and centuries BC

Bernd Helmle <mailings(at)oopsware(dot)de> writes:
> I stepped through the code in datetime.c and it seems the culprit here is
> DecodeDate(). It get's the date string from DecodeDateTime(), but without
> the 'BC' century notation. However, it then performs the following check

Yeah, I had just come to the same conclusion. It is premature for
DecodeDate to be trying to check this, because AFAICT there is no input
syntax in which it will be given both the date fields and the AD/BC
field. There is already checking code at the bottom of DecodeDateTime,
so it looks to me like DecodeDate's checks are simply redundant in that
code path. They aren't redundant in the calls from DecodeTimeOnly,
however. I think we should move the date range checks and BC adjustment
into a separate function ValidateDate() that is called from the bottom
of the outer loops in DecodeDateTime/DecodeTimeOnly.

The other issue is whether to throw error for year zero, rather than
silently interpreting it as 1 BC. I can't recall whether that behavior
was intentional at the time, but given our current rather strict
interpretation of date validity checking, it hardly seems like a good
idea now. What I suggest is that we throw error in 8.4 and beyond,
but not back-patch that change, so as to avoid introducing a behavioral
change in minor releases.

regards, tom lane