pgsql-server/ oc/src/sgml/datatype.sgml rc/bac ...

CVSROOT: /cvsroot
Module name: pgsql-server
Changes by: momjian(at)svr1(dot)postgresql(dot)org 03/12/20 11:32:55

Modified files:
doc/src/sgml : datatype.sgml
src/backend/commands: variable.c
src/backend/utils/adt: datetime.c
src/include : miscadmin.h
src/interfaces/ecpg/pgtypeslib: dt.h dt_common.c interval.c

Log message:
In my mind there were two categories of open issues
a) ones that are 100% backward (such as the comment about
outputting this format)
and
b) ones that aren't (such as deprecating the current
postgresql shorthand of
'1Y1M'::interval = 1 year 1 minute
in favor of the ISO-8601
'P1Y1M'::interval = 1 year 1 month.

Attached is a patch that addressed all the discussed issues that
did not break backward compatability, including the ability to
output ISO-8601 compliant intervals by setting datestyle to
iso8601basic.

Interval values can now be written as ISO 8601 time intervals, using
the "Format with time-unit designators". This format always starts with
the character 'P', followed by a string of values followed
by single character time-unit designators. A 'T' separates the date and
time parts of the interval.

Ron Mayer

momjian(at)svr1(dot)postgresql(dot)org (Bruce Momjian) writes:
> Attached is a patch that addressed all the discussed issues that
> did not break backward compatability,

Was this patch actually agreed to? I hadn't gotten around to looking
at it because I thought Peter was still objecting to the whole idea.

> including the ability to
> output ISO-8601 compliant intervals by setting datestyle to
> iso8601basic.

This is a horrid, horrid idea. Datestyle is already a complete mess
because it is being used to control several things; it should have been
two or possibly three GUC variables not one. Sticking in yet another
behavior is just not acceptable IMHO, especially when it's defined as
non-orthogonally as that.

Please revert that part of the patch and instead invent a new GUC
variable that's specifically for interval formatting.

BTW, I can tell without looking that the patch is deficient in
documentation; if it has effects on GUC variables, why is there no
mod in runtime.sgml?

regards, tom lane

Tom Lane wrote:
> momjian(at)svr1(dot)postgresql(dot)org (Bruce Momjian) writes:
> > Attached is a patch that addressed all the discussed issues that
> > did not break backward compatability,
>
> Was this patch actually agreed to? I hadn't gotten around to looking
> at it because I thought Peter was still objecting to the whole idea.
>
> > including the ability to
> > output ISO-8601 compliant intervals by setting datestyle to
> > iso8601basic.
>
> This is a horrid, horrid idea. Datestyle is already a complete mess
> because it is being used to control several things; it should have been
> two or possibly three GUC variables not one. Sticking in yet another
> behavior is just not acceptable IMHO, especially when it's defined as
> non-orthogonally as that.
>
> Please revert that part of the patch and instead invent a new GUC
> variable that's specifically for interval formatting.
>
> BTW, I can tell without looking that the patch is deficient in
> documentation; if it has effects on GUC variables, why is there no
> mod in runtime.sgml?

OK, I have backed out the patch. Peter did object, but I said I thought
the submitter had addressed the objections (backward compatibility) and
I asked for someone to tell me the objections, and no one replied, so I
assumed it was OK and it went through the standard patch application
process.

The patch is attached if someone wants to modify it to head in the
direction that most developers prefer.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

On Sat, 20 Dec 2003, Bruce Momjian wrote:
>Tom Lane wrote:
>>This is a horrid, horrid idea. Datestyle is already a complete mess
>> ...
>>Please revert that part of the patch and instead invent a new GUC
>>variable that's specifically for interval formatting.
>
> OK, I have backed out the patch. [...]

Short summary...

Before I try this, through email someone suggested yet a
different idea...

Would formatting functions for intervals such as...
to_iso8601basic_char(interval) -- return ISO-8601 basic fmt interval
to_iso8601basic_char(timestamp) -- return ISO-8601 basic fmt date/time
would be better than a new GUC variable?

Longer...

Tom Lane wrote:
>
> This is a horrid, horrid idea. Datestyle is already a complete mess
> because it is being used to control several things; it should have
> been two or possibly three GUC variables not one. Sticking in yet
> another behavior is just not acceptable IMHO, especially when it's
> defined as non-orthogonally as that.
>
> Please revert that part of the patch and instead invent a new GUC
> variable that's specifically for interval formatting.

First I just wanted to say how it ended up using datestyle...

In the earlier discussion when Andrew asked about a way of outputting
ISO-8601 Basic Format time intervals, the use of datestyle came up,
and noone objected to the use of datestyle at that point.

... Tom was suggesting:
http://archives.postgresql.org/pgsql-patches/2003-09/msg00122.php
TL>
TL> Perhaps call it "compact" or "terse" datestyle?

... and Peter suggested:
http://archives.postgresql.org/pgsql-patches/2003-09/msg00129.php
PE>
PE> iso8601
PE>
PE> Keep in mind that SQL itself is also a kind of ISO, so being
PE> more specific is useful.

Regarding the non-orthogonality, I was suspecting that most
applications that use ISO-8601 Basic Formats would use them
consistently for dates (19990131) and intervals (P1Y1M).
But I do see your point and agree this isn't a good solution.

If the developers would like separate GUC variables for
formatting dates vs intervals, I would be happy to do so.
On the other hand, if the idea of outputting ISO-8601
intervals is likely to be rejected anyway, I'd be happy
to not do it too. :-)

Or, how would people feel instead about formatting functions
to produce the various ISO-8601 formats?
to_iso8601basic_char(timestamp)
to_iso8601basic_char(interval)
I think this could be especially useful since the docs:
http://developer.postgresql.org/docs/postgres/functions-formatting.html
say that "to_char(interval, text)" is being deprecated,
meaning that converting intervals to formats other systems
accept will soon become harder.

Personally, though, I'm most interested in the input side.

I have an application that uses ISO-8601 Basic Format
for all it's time information (Dates, Times, and Intervals), and
wanted to load this information into PostgreSQL. I was happy
to see that Dates and Times loaded.

Unfortunately intervals did not.

A quick investigation showed that PostgreSQL currently has an
undocumented shorthand is similar but frustratingly different
from ISO-8601:
(i.e. '1Y1M'::interval means '1 year 1 minute' to PostgreSQL 7.3X,
while 'P1Y1M' means '1 year 1 month' to ISO-8601).
Even if nothing is done to the output side, allowing inputting
of such intervals would benefit me.

Would the developers prefer a patch allowing the inputting
of such intervals, and not support outputting at all?

> BTW, I can tell without looking that the patch is deficient in
> documentation; if it has effects on GUC variables, why is there no
> mod in runtime.sgml?

Point well taken. Before I submit any future patches I will try
to be more careful in this regard.

Ron

PS: The spec I'm referring to is ISO-8601... Section 5.5.4.2
http://www.webaugur.com/bibliotheca/standards/iso8601/154N362/index-25.html

Ron Mayer <ron(at)cheapcomplexdevices(dot)com> writes:
> Would formatting functions for intervals such as...
> to_iso8601basic_char(interval) -- return ISO-8601 basic fmt interval
> to_iso8601basic_char(timestamp) -- return ISO-8601 basic fmt date/time
> would be better than a new GUC variable?

I could see arguments for both --- in the timestamp world we have
DateStyle to control the default output format, plus to_char when you
want something different. I'd suggest continuing with your plan of
providing GUC control over the default interval format (just use a
separate GUC variable, please, for orthogonality). If you want to add
an explicit formatting function later, you can do that too.

BTW, I do not recall exactly why Karel wants to deprecate
to_char(interval), but I don't think it's because of any fundamental
objection to the notion of a formatting function. I think it was
because the present definition is badly designed and needs to be
replaced with a different API.

regards, tom lane

Added to TODO:

* Add GUC variable to allow output of interval values in ISO8601 format

---------------------------------------------------------------------------

Tom Lane wrote:
> Ron Mayer <ron(at)cheapcomplexdevices(dot)com> writes:
> > Would formatting functions for intervals such as...
> > to_iso8601basic_char(interval) -- return ISO-8601 basic fmt interval
> > to_iso8601basic_char(timestamp) -- return ISO-8601 basic fmt date/time
> > would be better than a new GUC variable?
>
> I could see arguments for both --- in the timestamp world we have
> DateStyle to control the default output format, plus to_char when you
> want something different. I'd suggest continuing with your plan of
> providing GUC control over the default interval format (just use a
> separate GUC variable, please, for orthogonality). If you want to add
> an explicit formatting function later, you can do that too.
>
> BTW, I do not recall exactly why Karel wants to deprecate
> to_char(interval), but I don't think it's because of any fundamental
> objection to the notion of a formatting function. I think it was
> because the present definition is badly designed and needs to be
> replaced with a different API.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 8: explain analyze is your friend
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073