Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Thanks. I wasn't sure about the error message returned when times are
> outside range; how about this instead? I'm not wedded to this approach
> -- I can return to yours if this one isn't liked -- but I think the
> more specific messages are better. I realize this is inconsistent with
> the make_date case which always displays the full date instead of
> specific fields, but I think it's more likely that someone is doing
> arithmetic to enter time fields than date. (Anyway maybe this is not an
> important enough issue to create more work for translators.)
I thought that last point was the most important one: doing it like that
would create more work for translators than it's worth. There's no reason
to think that people can't figure out which field it's unhappy about.
And what if more than one field is wrong? You'd be exposing an
implementation detail about the order in which the tests are made.
Another issue with the patch as submitted was that make_date with a
negative year value behaved unreasonably. I made it throw error, but
you could also argue that say "-44" ought to mean "44 BC". (Year zero
should be disallowed in any case, of course.) It would take a few
extra lines of code to do that.
Committed with those changes and some other cosmetic adjustments.
This doesn't really finish the TODO item, as that contemplated a
make_timestamp() function as well; but I don't see a reason not
to commit what we've got.
regards, tom lane