Re: [GENERAL] INTERVAL data type and libpq - what format?

I wrote:
> In a related example,

> regression=# select interval '123 11' day;
> interval
> ----------
> 134 days
> (1 row)

> we seem to be adding the 123 and 11 together. This is, um,
> surprising behavior ... I'd be inclined to think throwing an
> error is more appropriate.

I looked into this and found out why the code wasn't throwing an error
already: it seems to be a case of ancient bit-shaving. DecodeInterval
maintains an integer bitmask of field types it's seen already, and
reports an error if a new field's bit is already set in that mask.
However, DAY and WEEK are coded to share a bit, as are YEAR, DECADE,
CENTURY, and MILLENIUM; which means the code can't throw error for
multiple occurrences of any one of those field types. However, there
is room in the mask for all these fields to have their own bits, so
the attached proposed patch just does that.

The potential objections I can see to this patch are:

(1) it eats more than half of the remaining bits in the DTK_M() mask.
This doesn't seem to me to be a big problem --- it's not like people
are likely to come up with many more time intervals for us to support.
Also we could probably shave some bits by eliminating redundancies
if we had to. (For instance, I think IGNORE_DTF could safely be given
a code above 31 since there's no need for it to be in the bitmasks.)

(2) WEEK, DECADE, CENTURY, and MILLENNIUM are too generic to be used
as #define names without some kind of prefix or suffix. This is a real
risk, but considering the other #defines in this list have no prefix or
suffix, it seems inconsistent to put one on these. The buildfarm should
tell us soon enough if there's a real problem (the code does compile
cleanly for me). If we do have a problem, we should add a prefix/suffix
to all these macros at once; but I'd rather not do that as part of a
small bugfix.

Comments?

regards, tom lane