from:"Ron"


Brendan Jurd wrote:

On Sat, Nov 1, 2008 at 3:42 PM, Ron Mayer [EMAIL PROTECTED] wrote:

# Patch 3: cleanup.patch
 Fix rounding inconsistencies and refactor interval input/output code


Compile, testing and regression tests all checked out.
I've picked up on a few code style issues, fixes attached.

If I'm reading the patch correctly, it seems you've renamed two of the
functions in datetime.c:
 * AdjustFractionalSeconds = AdjustFractSeconds
 * AdjustFractionalDays = AdjustFractDays
To be frank, this doesn't really seem worthwhile.  It only saves five
characters in the name.  What was your reason for renaming them?


Otherwise many lines were over 80 chars long.
And it happened often enough I thought the shorter name
was less ugly than splitting the arguments in many of the
places where it's called.

I'm happy either way, tho.


I *was* going to question the inconsistent use of a space between the
pointer qualifier and the argument name, for example:

static char *
AddVerboseIntPart(char * cp, int value, char *units,
  bool * is_zero, bool *is_before)

But then I noticed that there's a lot of this going on in datetime.c,
some of it appears to predate your patches.  So I guess cleaning this
up in your function definitions would be a bit of a bolted-horse,
barn-door affair.  Unless you felt like cleaning it up throughout the
file, it's probably not worth worrying about.


I don't mindn cleaning it up; but someone could point me to which direction.


There are some very large-scale changes to the regression tests.  I'm
finding it difficult to grok the nature of the changes from reading a
diff.  If possible, could you post some quick notes on the
purpose/rationale of these changes?


Previously, much (but IIRC not quite all) of the interval output stuff
rounded to the hundredths place regardless of how many significant digits
there were.   So, for example, the interval 1.699 seconds would usually
appear as 1.70 for most but not all combinations of DateStyle
and HAVE_INT64_TIMESTAMP.  After a few discussions on the mailing list
I think people decided to simply show the digits that were

http://archives.postgresql.org/pgsql-hackers/2008-09/msg00998.php





--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: Updated interval patches (SQL std output, ISO8601 intervals, and interval rounding)


Brendan Jurd wrote:

On Wed, Nov 12, 2008 at 5:32 AM, Ron Mayer
[EMAIL PROTECTED] wrote:

Brendan Jurd wrote:

 * AdjustFractionalSeconds = AdjustFractSeconds
 * AdjustFractionalDays = AdjustFractDays

Otherwise many lines were over 80 chars long.
And it happened often enough I thought the shorter name
was less ugly than splitting the arguments in many of the
places where it's called.


Fair enough.  I don't have a strong opinion about that.


Cool.   If anyone does have an opinion on that, let me know
and I can change it whichever way people prefer.


There are some very large-scale changes to the regression tests.  ...

Previously, much (but IIRC not quite all) of the interval output stuff
rounded to the hundredths place regardless of how many significant digits
there were.


I understood about the rounding issues.  I was a bit confused by the
fact that the patch shows differences for an entire table of results
from the horology test, but I've just now realised that the whole
table is different because changing the output precision in some of
the rows has altered the column width.

Makes me wonder whether an unaligned psql output format would be a
better choice for the regression tests.  It would certainly make for
clearer diffs.  But that's a tangent for another email.


Yeah.  And that's what made the patch so big I had to gzip it.



I don't have any further gripes regarding this patch, apart from the
code style stuff I sent through in my previous post.  Did you have any
response to those?


Yup - you were right again.
Applied them and updated the website and attaching the patch.

I wonder what's the best way for myself to get out of those habits
in the future?  Some lint flags or similar?




cleanup.patch.gz
Description: GNU Zip compressed data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Interval code refactoring patch (Was: Re: [HACKERS] Patch for ISO-8601-Interval Input and output.)


Tom Lane wrote:

...failure case ... interval 'P-1Y-2M3DT-4H-5M-6';
This isn't the result I'd expect, and AFAICS the ISO spec does *not*
allow any unit markers to be omitted in the format with designators.


Yes, this is true. I see you already made the change.

Tom Lane wrote:

Applied with nontrivial revisions --- I fear I probably broke your third
patch again :-(.


No problem. It wasn't hard to update.  Attached is an updated patch (as
well as being updated on my website; but since it applies to HEAD it's
as easy to get here).

The bulk of the changes are in regression tests where rounding of
fractional seconds was changed as discussed up-thread back in Sep.

Seems I should also submit one more patch that merge the newest
DecodeInterval, EncodeInterval and related functions into
/ecpg/pgtypeslib/interval.c?

And beyond that, there's still some eccentricities with the interval code
(why's interval '1 year 1 year' ok but interval '1 second 1 second' not)
but I don't know if I'd do more harm or good trying to look at those.



cleanup.patch.gz
Description: GNU Zip compressed data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: Updated interval patches (SQL std output, ISO8601 intervals, and interval rounding)


Tom Lane wrote:

The original INT64 coding here is exact (at least for the common case
where fval is zero) but I'm not convinced that your revision can't
suffer from roundoff error.


Good point.  I'll study this tonight; and either try to make a patch
that'll be exact where fval's zero or try to come up with convincing
reasons that it's harmless.

Once this settles I suppose I should post a ECPG patch that's based
off of these Decode/Encode interval functions too?


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for ISO-8601-Interval Input and output.

2008-11-10 Thread Ron Mayer


Brendan Jurd wrote:

On Sat, Nov 8, 2008 at 2:19 AM, Ron Mayer [EMAIL PROTECTED] wrote:

Hmmm... Certainly what I had in datatype.sgml was wrong, but I'm
now thinking 5.5.4.2.1 and 5.5.4.2.2 would be the most clear?


Sorry, I don't understand what you mean by 5.5.4.2.1.  In the spec


Ah!   That 5.5.4.2.1 comes from apparently an old Oct 2000
draft version of the spec titled ISO/FDIS 8601.  (For now you can
see it here: http://0ape.com/postgres_interval_patches/ISO-FDIS-8601.pdf )

I'll fix all the links to point to the 2004 spec.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for ISO-8601-Interval Input and output.

2008-11-10 Thread Ron Mayer


Ron Mayer wrote:

Ah!   That 5.5.4.2.1 comes from apparently an old Oct 2000
draft version of the spec titled ISO/FDIS 8601.  (For now you can
see it here: http://0ape.com/postgres_interval_patches/ISO-FDIS-8601.pdf )

I'll fix all the links to point to the 2004 spec.


I updated my web site[1] with the latest version of this patch.

Main differences since last time
  * Merged with the IntervalStyle patch as it was checked into CVS.
  * Fixed references to consistently refer to the same version of
the ISO 8601 spec (ISO 8601:2004(E))

[1]  http://0ape.com/postgres_interval_patches/

PS: I realize that this patch makes datetime.c a bit longer that it needs
to be; and that some of the functions added in this patch can be used by the
other interval styles as well.   patch 3 that can be found on the same HTML
page does this refactoring.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle

2008-11-09 Thread Ron Mayer


Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

Brendan Jurd wrote:

...I did notice one final ...

Just checked in a fix to that one; and updated my website at
http://0ape.com/postgres_interval_patches/
and pushed it to my (hopefully fixed now) git server.


Applied with some revisions: I changed the rule for negating input
fields in SQL_STANDARD style as we discussed, made IntervalStyle into
an enum GUC, and did some pretty heavy editorialization on the docs
changes.


Cool.  Thanks!

Brendan and anyone else looking at these, I've done an initial merge
between the new CVS head and my other interval patches and pushed
them to my website link mentioned above; but I probably still need
to cleanup some of the docs merges.   I'm a bit tied up today; so
will probably cleanup the merge and study Tom's changes and post an
update when the other patches are ready to continue reviewing, probably
late tomorrow.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

Rather than forcing Postgres mode; couldn't it put a
set intervalstyle = [whatever the current interval style is]
in the dump file?


This would work for loading into a PG = 8.4 server, and fail miserably
for loading into pre-8.4 servers.  Even though we don't guarantee
backwards compatibility of dump files, I'm loath to adopt a solution
that will successfully load wrong data into an older server.


How is the case different from standard_conforming_strings; where ISTM
depending on postgresql.conf 8.4 will happily dump either

  SET standard_conforming_strings = off;
  ...
  INSERT INTO dumptest VALUES ('');

or

  SET standard_conforming_strings = on;
  ...
  INSERT INTO dumptest VALUES ('\\');

and AFAICT the latter will happily load wrong data into 8.1 with
only the error message

  ERROR:  parameter standard_conforming_strings cannot be changed

I imagine the use-case for standard_conforming_strings = 0  and
intervalstyle = sql_standard are petty similar as well.



I wonder if the best solution is that any dump file with
standard_conforming_strings=on include some SQL that will refuse
to load in pre-8.2 systems; and that any dump file with
intervalstyle=sql_standard refuse to load in pre-8.4 systems.
It seems pretty easy to add a sql fragment that checks version()
and put that in the beginning of a dump file that uses these GUCs
to enforce this.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:


(3) Put something into the dump file that will make the old
 server reject the file rather than successfully loading
 wrong data?   (Some if intervalstyle==std and version8.3
 abort loading the restore logic?)


There isn't any way to do that, unless you have a time machine in
your hip pocket.  The trouble with putting
set intervalstyle = something;
into the dump script is that older servers will (by default) report
an error on that line and keep right on chugging.


Not necessarily.  Couldn't we put

 select * from (select substring(version() from '[0-9\.]+') as version) as a
 join (select generate_series(0,1000)) as b on(version'8.4');

 set intervalstyle = something;

Or something similar in the dump file.



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

Tom Lane wrote:

The trouble is that older servers will (by default) report
an error on that line and keep right on chugging.



Not necessarily.  Couldn't we put



  select * from (select substring(version() from '[0-9\.]+') as version) as a
  join (select generate_series(0,1000)) as b on(version'8.4');
  set intervalstyle = something;


[ shrug... ]  It's still just one easily missable bleat.


Not here.

On my system it hangs forever on 8.3 or less and proceeds
harmlessly with 8.4.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Tom Lane wrote:

Oh, I see what you're trying to do.  The answer is no.  We're not going
to totally destroy back-portability of dumps, especially not for a
problem that won't even affect most people (negative intervals are
hardly common).



Similarly I wonder if pg_dump should add a fail if version  8.2 right
before it outputs
  SET standard_conforming_strings = on;
which IMHO is far more common than negative intervals and AFAICT
has the same risk.

For intervals, we would only add the fail code if intervalstyle was set
to one of the new interval styles (if the ISO8601 interval's accepted
it'll have the problem too).

For backward compatible patches, they could still have their
GUC settingse specify standard_conforming_strings and interval_style
values that are supported by whichever versions they want to support.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Tom Lane wrote:

BTW, I just noticed that CVS HEAD has a bug in reading negative SQL-spec
literals:
regression=# select interval '-2008-10'; 
regression=# select interval '--10';

Surely the latter must mean -10 months.  This is orthogonal to the
current patch ...


Perhaps the below patch fixes that?
(though line numbers probably won't match since this was based off
of the patched version)



*** a/src/backend/utils/adt/datetime.c
--- b/src/backend/utils/adt/datetime.c
***
*** 2879,2885  DecodeInterval(char **field, int *ftype, int nf, int range,
if (*cp != '\0')
return DTERR_BAD_FORMAT;
type = DTK_MONTH;
!   if (val  0)
val2 = -val2;
val = val * MONTHS_PER_YEAR + val2;
fval = 0;
--- 2879,2885 
if (*cp != '\0')
return DTERR_BAD_FORMAT;
type = DTK_MONTH;
!   if (field[0][0] == '-')
val2 = -val2;
val = val * MONTHS_PER_YEAR + val2;
fval = 0;
[5]lt:/home/ramayer/proj/pg/postgresql/src/backend/utils/adt%


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Tom Lane wrote:

Another thought here ... I'm looking at the sign hack
+ if (IntervalStyle == INTSTYLE_SQL_STANDARD 
 
and not liking it very much.  Yes, it does the intended thing for strict

SQL-spec input, but it seems to produce a bunch of weird corner cases
for non-spec input.  Consider [... many examples ...]

I'm inclined to think we need a more semantically-based instead of
syntactically-based rule.  For instance, if first field is negative and
no other field has an explicit sign, then force all fields to be = 0.
This would probably have to be applied at the end of DecodeInterval
instead of on-the-fly within the loop.

Thoughts?


I'll take a step back and think about that

Yes, at first glance I think that approach is better; but we'd need
to make sure not to apply the rule too enthusiastically on traditional
postgres intervals; or worse, ones that mix sql standardish and postgres
values  For example
  dish=# select interval '-1 1:1 1 years';
   interval
  --
   1 year -1 days +01:01:00
  (1 row)
that 8.3 accepts. (or do we not care about those)?

In some ways I wonder if we should have 2 totally separate parsing
one for the SQL standard ones, and one for the postgres.
That would avoid some other confusing inputs like:
  select interval '-00-01 1 years';
  select interval '1-1 hours';
  select interval '1:1 years';
  select interval '1 hours 1-1 1 years'
that are currently accepted.



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for ISO-8601-Interval Input and output.


Brendan Jurd wrote:

On Fri, Nov 7, 2008 at 3:35 AM, Ron Mayer [EMAIL PROTECTED] wrote:

I think I updated the web site and git now, and
'P-00-01' is now accepted.   It might be useful if
someone double checked my reading of the spec, tho.


I've tested out your latest revision and read the spec more closely,
and pretty much everything works as expected. ...
I agree with your interpretation of the spec, it clearly says that 'T'
can be omitted when there are no time components. ...
The examples in Annex B confirm this.


Cool.  Thanks.


I did run into one potential bug:
postgres=# select interval 'P0001';
 ...
Whereas, I expected to get '1 year', since the format allows you to
omit lower-order components from right-to-left:
P0001-01-01 = 1 year 1 month 1 day
P0001-01 = 1 year 1 month
P0001 = should be 1 year?


Indeed, that's right.   Thanks for catching another one.
I just checked in (to my git) a patch that I believe fixes it.

regression=# select interval 'P0001',interval 'P0001',interval 'PT01';
 interval | interval | interval
--+--+--
 1 year   | 1 year   | 01:00:00
(1 row)


On the documentation front, I have a few final cleanups to suggest
(patch attached).
 * After giving the spec a closer look, I thought that 4.4.3.2 and
4.4.3.3 were the proper spec references to use for the two formats.


Hmmm... Certainly what I had in datatype.sgml was wrong, but I'm
now thinking 5.5.4.2.1 and 5.5.4.2.2 would be the most clear?

Totally agree with the rest of your docs changes and applied those.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Tom Lane wrote:


I've started reviewing this patch for commit, and I find myself a bit
disturbed by its compatibility properties.  The SQL_STANDARD output
style is simply ambiguous: what is meant by
-1 1:00:00
?  What you get from that will depend on the intervalstyle setting at
the recipient.


Nope.  The SQL Standard style avoids the ambiguity by following
the SQL Standard's rules when the input value complied with the
standard's restrictions on intervals.

For example - given the sql standard compliant value of negative
one days and negative one hours you get -1 1;00:00.

If you give it a non-sql-standard-compliant value like negative
one days plus one hours it will force outputting all the signs
both positive and negative:

regression=# select interval '-1 days +1 hours';
 interval
--
 +0-0 -1 +1:00:00
(1 row)


I agree that there's an ambiguity on input - in much the same way
that date order can affect ambiguous inputs.


 Either of the traditional Postgres styles are
non-ambiguous and will be interpreted correctly regardless of receiver's
intervalstyle --- in particular, Postgres mode always puts an explicit
sign on the time part if the days or months part was negative.  What
this means is that SQL_STANDARD mode is unsafe for dumping data, and


So long as the SQL Standard style is chosen both on dumping and loading,
I think it will preserve any values given to it.


*pg_dump had better force Postgres mode*.  We can certainly do that with
a couple more lines added to the patch, but it's a bit troublesome that
we are boxed into using a nonstandard dump-data format until forever.

I don't immediately see any way around that, though.  Anyone have a
bright idea?


Are you concerned about someone dumping in SQL_STANDARD mode and
then importing in POSTGRES mode?   If so, how's the similar case
handled with date order?


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Ron Mayer wrote:

Tom Lane wrote:

*pg_dump had better force Postgres mode*.  We can certainly do that with
a couple more lines added to the patch, but it's a bit troublesome that
we are boxed into using a nonstandard dump-data format until forever.


Ok.  I see that is the concern..

Rather than forcing Postgres mode; couldn't it put a
set intervalstyle = [whatever the current interval style is]
in the dump file?

That doesn't force us to using a nonstandard dump-data format (except
for the nonstandard set intervalstyle line).


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Tom Lane wrote:


ISO date format is read the same regardless of recipient's datestyle,
so pg_dump solves this by forcing the dump to be made in ISO style.
The corresponding solution for intervals will be to dump in POSTGRES
style, not SQL_STANDARD style, which seems a bit unfortunate.


[reading pg_dump.c now]

I wonder if it could be similar to standard_conforming_strings
where it appears to be reading the current value and setting it
to whatever the user chose in the beginning of pg_dump. Then we
could dump in whichever intervalstyle the user prefers.   Or,
for 8.4+ dumps we could even force set intervalstyle = sql_standard;
in the top of the dump file.  For dumps of 8.3 or less we'd need
the non-standard style anyway it seems.

If this seems sane, I can try experimenting with it tonight.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

Rather than forcing Postgres mode; couldn't it put a
set intervalstyle = [whatever the current interval style is]
in the dump file?


This would work for loading into a PG = 8.4 server, and fail miserably
for loading into pre-8.4 servers.  Even though we don't guarantee
backwards compatibility of dump files, I'm loath to adopt a solution
that will successfully load wrong data into an older server.


'k.
So the options seem to be:

(1) Don't output a SQL-standard interval literal for the
value negative one days and negative one hours; perhaps
by sticking an extra '+' sign in there?

(2) Force pg_dump to a non-standard mode, at least until 8.3's
deprecated in many years?
After that, pg_dump can use any intervalstyle so long as
it says which one it uses.

(3) Put something into the dump file that will make the old
server reject the file rather than successfully loading
wrong data?   (Some if intervalstyle==std and version8.3
abort loading the restore logic?)

I don't much like the first one, because it seems we're oh-so-close
to meeting the standard.

I don't know how to do the third one; but if pg_dump had
an assert(version=8.4) feature it might be useful if
we ever had other non-backward-compatible changes.  pg_dump
would only put that assert-like-thing in the dump file if
the sql_standard mode (or iso mode, if that gets approved)
was chosen.

The second one doesn't really seem that scary to me; since
the uglyness can go away when we eventually stop restoring
into 83.



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for ISO-8601-Interval Input and output.

2008-11-06 Thread Ron Mayer


Brendan Jurd wrote:

I've applied them with a couple minor changes.

* If ISO 8601 5.5.3.1.d's statement The designator T shall be
absent if all of the time components are absent. also applies
to 5.5.4.2.2; then I think the 'T' needed to be inside the
optional tags, so I moved it there.  The link to the spec's
below[1].


Hmm, okay.  When I was running my tests in psql I came away with the
impression that the T was required in the alternative format.  I
might be mistaken.  I'll run some further tests a little later on.


Indeed that's a bug in my code; where I was sometimes
requiring the 'T' (in the ISO8601 alternative format) and
sometimes not (in the ISO8601 format from 5.5.4.2.1).

Below's a test case.   If I read the spec[1] right both of those
should mean 1 day.  I'll update git and post a new patch now.
If people think I read the specs wrong, I'll undo this change
and fix the docs.


==
[2]lt:/home/ramayer/proj/pg% ./psql regression
psql (8.4devel)
Type help for help.

regression=# select interval 'P1D';
 interval
--
 1 day
(1 row)

regression=# select interval 'P-00-01';
ERROR:  invalid input syntax for type interval: P-00-01
LINE 1: select interval 'P-00-01';
^
==

[1]
http://isotc.iso.org/livelink/livelink/4021199/ISO_8601_2004_E.zip?func=doc.Fetchnodeid=4021199

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for ISO-8601-Interval Input and output.

2008-11-06 Thread Ron Mayer


Ron Mayer wrote:

Brendan Jurd wrote:
 'T' ... optional 


Indeed that's a bug in my code; where I was sometimes
requiring the 'T' (in the ISO8601 alternative format) and
sometimes not (in the ISO8601 format from 5.5.4.2.1).

Below's a test case.   If I read the spec[1] right both of those
should mean 1 day.  I'll update git and post a new patch now.
If people think I read the specs wrong, I'll undo this change
and fix the docs.


I think I updated the web site and git now, and
'P-00-01' is now accepted.   It might be useful if
someone double checked my reading of the spec, tho.


[1]
http://isotc.iso.org/livelink/livelink/4021199/ISO_8601_2004_E.zip?func=doc.Fetchnodeid=4021199 




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle

2008-11-05 Thread Ron Mayer


Brendan Jurd wrote:

On Wed, Nov 5, 2008 at 7:34 AM, Ron Mayer [EMAIL PROTECTED] wrote:

Brendan Jurd wrote:

...new interval

Review of the other two patches coming soon to a mail client near you.



Oh - and for review of the next patch, ISO 8601's spec would no doubt
be useful.

I think this is the right place to get it:
http://isotc.iso.org/livelink/livelink/4021199/ISO_8601_2004_E.zip?func=doc.Fetchnodeid=4021199

Is there anywhere in the comments, docs, or wiki where such links
and/or excerpts from specs belong?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for ISO-8601-Interval Input and output.

2008-11-05 Thread Ron Mayer

Brendan Jurd wrote:

Reviewing this patch now; I'm working from the 'iso8601' branch in
... I thought I'd post a patch of my own (against your branch)
and accompany it with a few explanatory notes.

Wow thanks! That's very helpful (though it might have been more
fair to your time if you just kicked it back to me saying rewrite
the docs so they make sense)!

I've applied them with a couple minor changes.

* If ISO 8601 5.5.3.1.d's statement The designator T shall be
absent if all of the time components are absent. also applies
to 5.5.4.2.2; then I think the 'T' needed to be inside the
optional tags, so I moved it there. The link to the spec's
below[1].

* There was a sect2 that the patch changed to a sect3, and
with that change I get an error:
openjade:datatype.sgml:2306:31:E: document type does not allow element SECT3
here
so I changed it back to a sect2.

You can see my changes to your patch on gitweb here: http://tinyurl.com/6crks6
and see how they got applied after your patch here http://tinyurl.com/685hla

I think I've updated my website, my git, and the cleanup patch to include
these changes now.

Most of these changes are of a highly subjective nature. I think they
are improvements, but someone else might prefer the way it was before
I got my hands dirty. Please consider the changes in my patch a set
of suggestions for making the documentation on this feature a little
easier to digest. You're welcome to take or leave them as you see
fit.

They're clearly improvements. I'm a total novice when it comes to
writing docs.

[1]
http://isotc.iso.org/livelink/livelink/4021199/ISO_8601_2004_E.zip?func=doc.Fetchnodeid=4021199

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Brendan Jurd wrote:

...Sep 18, 2008... Ron Mayer [EMAIL PROTECTED] wrote:

  The attached patch
(1) adds a new GUC called IntervalStyle that decouples interval
output from the DateStyle GUC, and
(2) adds a new interval style that will match the SQL standards
for interval literals when given interval data that meets the
sql standard (year-month or date-time only; and no mixed sign).


I've been assigned to do an initial review of your interval patches.
I'm going to be reviewing them one at a time, starting with this one
(the introduction of the new IntervalStyle GUC).


Great!  Thanks much!


I grabbed the latest version of the patch from the URL posted up on
the CF wiki page:
http://0ape.com/postgres_interval_patches/stdintervaloutput.patch

Nice site you've got set up for the patches, BTW.  It certainly makes
it all a lot more approachable.


Ah. If you're using GIT, you might find it more convenient to pull/merge
from
  http://git.0ape.com/postgresql/
or browse through gitweb:
  http://git.0ape.com/?p=postgresql;a=shortlog;h=refs/heads/cleanup
  http://git.0ape.com/git-browser/by-commit.html?r=postgresql
though this is the first time I've set up gitweb so it might have rough edges.


The patch applied cleanly to the latest version of HEAD in the git
repository.  I was able to build both postgres and the documentation
without complaint on x86_64 gentoo.

When I ran the regression tests, I got one failure in the new interval
tests.  Looks like the nonstandard extended format gets a bit
confused when the seconds are negative:


Ah yes.   Let me guess, HAVE_INT64_TIMESTAMP was defined.  I believe
the later refactoring patch also avoids that bug; but yes, I obviously
should have had it working in this patch.

This fix was simple (can be seen on gitweb here: http://tinyurl.com/5fxeyw)
and I think I've pushed the updated patches to my website.

Once I fix the stylistic points you mentioned below I'll post
the resulting patch to the mailing list.


Otherwise, the feature seemed to behave as advertised.  I tried
throwing a few bizarre intervals at it, but didn't manage to break
anything.

The C code has some small stylistic inconsistencies
...documentation...some
minor stylistic and spelling cleanups I would suggest.


Totally agree with all your style suggestions.   Will send an update
a bit later today.



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Ah.  And one final question regarding functionality.

It seems to me that the last remaining place where we input
a SQL-2008 standard literal and do something different from
what the standard suggests is with the string:
  '-1 2:03:04'
The standard seems to say that the - affects both the
days and hour/min/sec part; while PostgreSQL historically,
and the patch as I first submitted it only apply the negative
sign to the days part.

IMHO when the IntervalStyle GUC is set to sql_standard,
it'd be better if the parsing of this literal matched the
standard.  We already have the precedent where DateStyle
is used to interpret otherwise ambiguous output.

If the IntervalStyle is set to anything other than sql_standard
we'll keep parsing them the old way; so I think backwards
compatibility issues would be minimized.   And those
using the sql_standard mode are likely to be standards
fanatics anyway, and would probably appreciate following the
standard rather than the backward compatible mode.

  Thoughts?
  I have a version of each alternative working here,
  and I'd be happy to submit the final patch either way.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

Ah.  And one final question regarding functionality.
It seems to me that the last remaining place where we input
a SQL-2008 standard literal and do something different from
what the standard suggests is with the string:
   '-1 2:03:04'
The standard seems to say that the - affects both the
days and hour/min/sec part; while PostgreSQL historically,
and the patch as I first submitted it only apply the negative
sign to the days part.



IMHO when the IntervalStyle GUC is set to sql_standard,
it'd be better if the parsing of this literal matched the
standard.


Then how would you input a value that had different signs for the
day and the h/m/s?  I don't think you can't is an acceptable
answer there, because it would mean that interval_out has to fail
on such values when IntervalStyle is sql_standard.  Which is
very clearly not gonna do.


In the patch I submitted:
-1 +2:03:04 always means negative day, positive hours/min/sec
+1 -2:03:04 always means positive day, negative hours/min/sec

When given a non-standard interval value, EncodeInterval is
always outputting all the signs (+ and -) to force it
to be unambiguous.

-- test a couple non-standard interval values too
SELECT  interval '1 years 2 months -3 days 4 hours 5 minutes 6.789 seconds',
- interval '1 years 2 months -3 days 4 hours 5 minutes 6.789 seconds';
   interval   |   ?column?
--+--
 +1-2 -3 +4:05:06.789 | -1-2 +3 -4:05:06.789
(1 row)



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Brendan Jurd wrote:

...Sep 18, 2008...Ron Mayer [EMAIL PROTECTED] wrote:

(1) ...GUC called IntervalStyle...
(2) ...interval style that will match the SQL standards...


...an initial review...

When I ran the regression tests, I got one failure in the new interval


Fixed, and I did a bit more testing both with and without HAVE_INT64_TIMESTAMP.


The C code has some small stylistic inconsistencies; ...
... spaces around binary operators are missing (e.g., (fsec0)).


Thanks.  Fixed these.


...function calls missing the space after the argument separator...


I think I fixed all these now too.


The new documentation is good in terms of content, but there are some
minor stylistic and spelling cleanups I would suggest.
...variously...SQL standard, SQL-standard and SQL Standard...


Got it.  There are a few inconsistencies elsewhere in the file
talking about other data types.  I wonder if I should fix those
as well.


These sentences in datatype.sgml are a bit awkward ...
I would go with something more along the lines of...


Yes.  Thanks for the better wording.

I don't think old releases is specific enough.  


Yup - fixed that too.


That's all the feedback I have for the moment.  I hope you found my
comments helpful.  I'll be setting the status of this patch to
Returned with Feedback and wait for your responses before I move
forward with reviewing the other patches.


Great.   I've tried to update the style on my remaining patches as well.


In addition, I've added to the docs describing how I use
explicit '+' and '-' signs to disambiguate the mixed-sign
non-standard intervals when in the sql_standard mode.


As before the 3 patches are at: http://0ape.com/postgres_interval_patches/
and http://git.forensiclogic.com/postgresql/ and
http://git.forensiclogic.com/?p=postgresql;a=shortlog;h=refs/heads/cleanup

I'm attaching the patch dealing with sql standard intervals here for the 
archives.

*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***
*** 4013,4018  SET XML OPTION { DOCUMENT | CONTENT };
--- 4013,4056 
/listitem
   /varlistentry
  
+  varlistentry id=guc-intervalstyle xreflabel=IntervalStyle
+   termvarnameIntervalStyle/varname (typestring/type)/term
+   indexterm
+primaryvarnameIntervalStyle/ configuration parameter/primary
+   /indexterm
+   listitem
+para
+ Sets the display format for interval values. 
+ The value literalsql_standard/ will produce
+ output matching acronymSQL/acronym standard
+ interval literals for values that conform to the 
+ acronymSQL/acronym standard (either year-month 
+ only or date-time only; and no mixing of positive 
+ and negative components).
+ 
+ The value literalpostgres/ will produce output
+ matching PostgreSQL releases prior to 8.4
+ when the xref linkend=guc-datestyle
+ parameter was set to literalISO/.
+ 
+ The value literalpostgres_verbose/ will produce output
+ matching PostgreSQL releases prior to 8.4
+ when the xref linkend=guc-datestyle
+ parameter was set to literalSQL/.
+/para
+para
+ The IntervalStyle GUC also affects the interpretation
+ of one ambiguous interval literal input.  In SQL 2008
+ the negative sign in the interval literal '-1 2:03:04'
+ applies to both the days and hour/minute/second parts.
+ PostgreSQL traditionally only applied the negative
+ sign to the days part.  If IntervalStyle is set to
+ literalsql_standard/literal it will follow the standard
+ otherwise it uses the traditional postgres interpretation.
+/para
+   /listitem
+  /varlistentry
+ 
   varlistentry id=guc-timezone xreflabel=timezone
termvarnametimezone/varname (typestring/type)/term
indexterm
*** a/doc/src/sgml/datatype.sgml
--- b/doc/src/sgml/datatype.sgml
***
*** 1962,1968  January 8 04:05:06 1999 PST
a combination of years and months can be specified with a dash;
for example literal'200-10'/ is read the same as literal'200 years
10 months'/.  (These shorter forms are in fact the only ones allowed
!   by the SQL standard.)
   /para
  
   para
--- 1962,1968 
a combination of years and months can be specified with a dash;
for example literal'200-10'/ is read the same as literal'200 years
10 months'/.  (These shorter forms are in fact the only ones allowed
!   by the acronymSQL/acronym standard.)
   /para
  
   para
***
*** 2213,2218  January 8 04:05:06 1999 PST
--- 2213,2310 
  /para
 /sect2
  
+sect2 id=interval-output
+ titleInterval Output/title
+ 
+ indexterm
+  primaryinterval/primary
+  secondaryoutput format/secondary
+  seealsoformatting/seealso
+ /indexterm
+ 
+ para

Re: [HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Brendan Jurd wrote:

The changes to the documentation all look good.  I did notice one
final typo that I think was introduced in the latest version.
doc/src/sgml/datatype.sgml:2270 has Nonstandardrd instead of
Nonstandard.


Just checked in a fix to that one; and updated my website at
http://0ape.com/postgres_interval_patches/
and pushed it to my (hopefully fixed now) git server.

If this'll be the final update to this patch should I be
posting it to the mailing list too for the archives?


But, apart from that I have no further feedback.
I will sign off on this one and mark it Ready for committer in the commitfest.


Cool.   I'm not sure if anyone still wants to weigh in on the
approach I took for mixed-sign intervals, where '-1 2:03:04'
gets interpreted differently  depending on the date style,
and '-1 +2:03:04' and '-1 -2:03:04' are the way I'm using
to disambiguate them.


Review of the other two patches coming soon to a mail client near you.


Feel free to do them one-at-a-time too; since no doubt any issues
with the first one will probably affect the second one too.

I think I updated the other patches for the missing whitespace
style issues my first patch had; but no doubt there could be
other bad habits I have as well.

   Ron




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: Updated interval patches (SQL std output, ISO8601 intervals, and interval rounding)

2008-10-31 Thread Ron Mayer


Ron Mayer wrote:
 Tom Lane wrote:
 In fact, given that we are now
 somewhat SQL-compliant on interval input, a GUC that selected
 PG traditional, SQL-standard, or ISO 8601 interval output format seems
 like it could be a good idea.

Attached are updated versions of the Interval patches (SQL-standard interval
output, ISO8601 intervals, and interval rounding) I posted earlier upthread.
I mostly brought it up-to-date with HEAD, cleaned up comments and regression
tests, and fixed a couple bugs.  [Sorry if people get this twice.  I tried
attaching all 4 patches earlier today, but didn't notice it on the
list perhaps because of the combined size.]

# Patch 1: stdintervaloutput.patch
   SQL Standard Interval Literal Output

   Description: This patch adds an IntervalStyle GUC to control
   the style of intervals. Previously the interval style was a
   side-effect of the DateStyle GUC. IntervalStyle can be set to
   sql_standard to output SQL Standard Interval Literals.

   Reason for the patch: Now that we support SQL-standard interval
   inputs, it's nice to be able to output intervals in that style as well.

During the commit-fest I'll post versions of these that are regularly
synced with CVS HEAD here:  http://0ape.com/postgres_interval_patches/

*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***
*** 4016,4021  SET XML OPTION { DOCUMENT | CONTENT };
--- 4016,4043 
/listitem
   /varlistentry
  
+  varlistentry id=guc-intervalstyle xreflabel=IntervalStyle
+   termvarnameIntervalStyle/varname (typestring/type)/term
+   indexterm
+primaryvarnameIntervalStyle/ configuration parameter/primary
+   /indexterm
+   listitem
+para
+ Sets the display format for interval values. 
+ The value literalsql_standard/ will output SQL Standard
+ strings when given intervals that conform to the SQL
+ standard (either year-month only or date-time only; and no
+ mixing of positive and negative components).
+ The value literalpostgres/ will output intervals in
+ a format that matches what old releases had output when
+ the DateStyle was set to literal'ISO'/.
+ The value literalpostgres_verbose/ will output intervals in
+ a format that matches what old releases had output when
+ the DateStyle was set to literal'SQL'/.
+/para
+   /listitem
+  /varlistentry
+ 
   varlistentry id=guc-timezone xreflabel=timezone
termvarnametimezone/varname (typestring/type)/term
indexterm
*** a/doc/src/sgml/datatype.sgml
--- b/doc/src/sgml/datatype.sgml
***
*** 2213,2218  January 8 04:05:06 1999 PST
--- 2213,2305 
  /para
 /sect2
  
+sect2 id=interval-output
+ titleInterval Output/title
+ 
+ indexterm
+  primaryinterval/primary
+  secondaryoutput format/secondary
+  seealsoformatting/seealso
+ /indexterm
+ 
+ para
+  The output format of the interval types can be set to one of the
+  three styles literalsql_standard/, 
+  literalpostgres/, or literalpostgres_verbose/.
+  The default is the literalpostgres/ format.  
+  xref
+  linkend=interval-style-output-table shows examples of each
+  output style.
+ /para
+ 
+ para
+  The literalsql_standard/ style will output SQL standard
+  interval literal strings where the value of the interval
+  value consists of only a year-month component or a datetime
+  component (as required by the sql standard).   For an interval
+  containing both a year-month and a datetime component, the
+  output will be a SQL Standard unquoted year-month literal
+  string joined to a SQL Standard unquoted datetime literal
+  string with a space in between.
+ /para
+ 
+ para
+  The literalpostgres/ style will output intervals that match
+  the style PostgreSQL 8.3 outputed when the xref linkend=guc-datestyle
+  parameter was set to literalISO/.
+ /para
+ 
+ para
+  The literalpostgres_verbose/ style will output intervals that match
+  the style PostgreSQL 8.3 outputed when the xref linkend=guc-datestyle
+  parameter was set to literalSQL/.
+ /para
+ 
+  table id=interval-style-output-table
+ 	   titleInterval Style Example/title
+ 	   tgroup cols=2
+ 		thead
+ 		 row
+ 		  entryStyle Specification/entry
+ 		  entryYear-Month Interval/entry
+ 		  entryDateTime Interval/entry
+ 		  entryNonstandardrd Extended Interval/entry
+ 		 /row
+ 		/thead
+ 		tbody
+ 		 row
+ 		  entrysql_standard/entry
+ 		  entry1-2/entry
+ 		  entry3 4:05:06/entry
+ 		  entry-1-2 +3 -4:05:06/entry
+ 		 /row
+ 		 row
+ 		  entrypostgres/entry
+ 		  entry1 year 2 mons/entry
+ 		  entry3 days 04:05:06/entry
+ 		  entry-1 year -2 mons +3 days -04:05:06/entry
+ 		 /row
+ 		 row
+ 		  entrypostgres_verbose/entry
+ 		  entry@ 1 year 2 mons/entry
+ 		  entry@ 3 days 4 hours 5 mins 6 secs

Re: [HACKERS] Re: Updated interval patches (SQL std output, ISO8601 intervals, and interval rounding)

2008-10-31 Thread Ron Mayer


Ron Mayer wrote:

Ron Mayer wrote:
  Tom Lane wrote:
  In fact, given that we are now
  somewhat SQL-compliant on interval input, a GUC that selected
  PG traditional, SQL-standard, or ISO 8601 interval output format seems
  like it could be a good idea.

Attached are updated versions of the Interval patches ...



# Patch 2:
   ISO 8601 Formatted Interval Input and Output

   This patch adds another IntervalStyle 'iso_8601' to output ISO 8601
   Time Intervals of the format with designators. These are a bit
   more flexible than Sql Standard intervals in that (like postgres)
   they can express both years and days in the same interval value.

   Reason for the patch:SQL Standard Intervals are limited compared to
   postgres in what they allow (no mixed year-month and day-time
   components). ISO8601 intervals allow such intervals and are easier
   for machines to parse than the traditional postgres formats.

   This patch depends on the IntervalStyle patch mentioned above.
*** a/doc/src/sgml/datatype.sgml
--- b/doc/src/sgml/datatype.sgml
***
*** 1975,1980  January 8 04:05:06 1999 PST
--- 1975,1996 
   /para
  
   para
+   Alternatively, typeinterval/type values can be written as 
+   ISO 8601 time intervals, using the Format with time-unit designators,
+   or PnYnMnDTnHnMnS.   This format always starts with the character 
+   literal'P'/, followed by a string of values followed by single
+   character time-unit designators.  A literal'T'/ separates the 
+   date and time parts of the interval.
+   In this format, literal'n'/ gets replaced by a number, and 
+literalY/ represents years, 
+literalM/ (in the date part) months,
+literalD/ months,
+literalH/ hours,
+literalM/ (in the time part) minutes,
+and literalS/ seconds.
+  /para
+ 
+  para
Internally typeinterval/ values are stored as months, days,
and seconds. This is done because the number of days in a month
varies, and a day can have 23 or 25 hours if a daylight savings
***
*** 2224,2230  January 8 04:05:06 1999 PST
  
  para
   The output format of the interval types can be set to one of the
!  three styles literalsql_standard/, 
   literalpostgres/, or literalpostgres_verbose/.
   The default is the literalpostgres/ format.  
   xref
--- 2240,2246 
  
  para
   The output format of the interval types can be set to one of the
!  four styles literalsql_standard/, literaliso_8601/, 
   literalpostgres/, or literalpostgres_verbose/.
   The default is the literalpostgres/ format.  
   xref
***
*** 2244,2249  January 8 04:05:06 1999 PST
--- 2260,2281 
  /para
  
  para
+  The literaliso_8601/ style will output ISO 8601 
+  time intervals using the format with time-unit designators
+  This format always starts with the character 
+   literal'P'/, followed by a string of values followed by single
+   character time-unit designators.  A literal'T'/ separates the 
+   date and time parts of the interval.
+   In this format, literal'n'/ gets replaced by a number, and 
+literalY/ represents years, 
+literalM/ (in the date part) months,
+literalD/ months,
+literalH/ hours,
+literalM/ (in the time part) minutes,
+and literalS/ seconds.
+ /para
+ 
+ para
   The literalpostgres/ style will output intervals that match
   the style PostgreSQL 8.3 outputed when the xref linkend=guc-datestyle
   parameter was set to literalISO/.
***
*** 2274,2279  January 8 04:05:06 1999 PST
--- 2306,2317 
  		  entry-1-2 +3 -4:05:06/entry
  		 /row
  		 row
+ 		  entryiso_8601/entry
+ 		  entryP1Y2M/entry
+ 		  entryP3DT4H5M6/entry
+ 		  entryP-1Y-2M3DT-4H-5M-6/entry
+ 		 /row
+ 		 row
  		  entrypostgres/entry
  		  entry1 year 2 mons/entry
  		  entry3 days 04:05:06/entry
*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c
***
*** 248,253  assign_intervalstyle(const char *value, bool doit, GucSource source)
--- 248,257 
  	{
  		newIntervalStyle = INTSTYLE_SQL_STANDARD;
  	}
+ 	else if (pg_strcasecmp(value, iso_8601) == 0)
+ 	{
+ 		newIntervalStyle = INTSTYLE_ISO_8601;
+ 	}
  	else
  	{
  		ereport(GUC_complaint_elevel(source),
*** a/src/backend/utils/adt/datetime.c
--- b/src/backend/utils/adt/datetime.c
***
*** 2723,2728  DecodeSpecial(int field, char *lowtoken, int *val)
--- 2723,2865 
  }
  
  
+ /*
+  * Small helper functions to avoid cutpaste code in DecodeIso8601Interval
+  */
+ static void 
+ adjust_fractional_seconds(double fval,struct pg_tm * tm, fsec_t *fsec, int scale)
+ {
+ 	int	sec;
+ 	if (fval == 0) return;
+ 	fval   *= scale;
+ 	sec	= fval;
+ 	tm-tm_sec += sec;
+ #ifdef HAVE_INT64_TIMESTAMP
+ 	*fsec  += rint((fval - sec) * 100);
+ #else

Re: [HACKERS] Re: Updated interval patches (SQL std output, ISO8601 intervals, and interval rounding)

2008-10-31 Thread Ron Mayer


Ron Mayer wrote:

Ron Mayer wrote:

Ron Mayer wrote:
  Tom Lane wrote:
  In fact, given that we are now
  somewhat SQL-compliant on interval input, a GUC that selected
  PG traditional, SQL-standard, or ISO 8601 interval output format 
seems

  like it could be a good idea.

Attached are updated versions of the Interval patches ...


# Patch 3: cleanup.patch
  Fix rounding inconsistencies and refactor interval input/output code

  This patch removes a lot of copy  paste with gratuitous rounding
  inconsistencies in the old interval code. This patch refactors it to save
  about 300 lines in datetime.c, and by reusing code (instead of the
  near-copy-paste that was there before) helps insure that rounding
  inconsistancies are avoided.

  It removes almost as much code as the other two patches added.

  This patch applies on top of patch 1 and 2 posted up-thread..

My apologies if you got these twice, my mailer seems to be rejecting
even slightly large attachments so I tried a couple times.

During the commit-fest I'll post versions of these that are regularly
synced with CVS HEAD here:  http://0ape.com/postgres_interval_patches/
along with a combined patch that includes all 3 of these in a single patch.


cleanup.patch.gz
Description: GNU Zip compressed data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] new correlation metric

2008-10-27 Thread Ron Mayer


Jeff Davis wrote:

Currently, we use correlation to estimate the I/O costs of an index
scan. However, this has some problems:


It certainly helps some cases.

Without the patch, the little test script below ends up picking the
third fastest plan (a seq-scan) instead of a faster bitmapscan, or
an even faster-than-that indexscan for the query below.
With the patch, it finds the fastest index scan.

Without Patch Estimated_cost Actual_Time
Index Scan39638.36   331ms
Bitmap Scan   22218.43   415ms
Seq Scan  20125.83   595ms

With PatchEstimated_cost Actual_Time
Index Scan17684.18   333ms
Bitmap Scan   22110.60   400ms
Seq Scan  20117.51   573ms

I was somewhat surprised that the bitmap cost estimates didn't
also change much.  Wouldn't the estimated # of data blocks
read for the bitmap be roughly the same as for the index?

And yes, I know that table's a contrived one that is almost
ideal for this patch - but I have some large clustered-by-zip
address tables where I can find queries that show similar results.

Back in 8.0 I cared a lot since I had a number of real-world
queries picking Seq-Scans instead of Index-Scans.   With 8.3,
though, AFAICT the vast majority of my similar real-world queries
pick the bitmap scans which in practice are pretty close in speed
to the index scans.




==
-- [1] Test script variation from this 2005 thread:
--http://archives.postgresql.org/pgsql-hackers/2005-02/msg00298.php

create temporary table tmp1mil as
  select * from
 (select generate_series as a from generate_series(0,9)) as a,
 (select generate_series as b from generate_series(0,9)) as b,
 (select generate_series as c from generate_series(0,9)) as c,
 (select generate_series as d from generate_series(0,9)) as d,
 (select generate_series as e from generate_series(0,9)) as e,
 (select generate_series as f from generate_series(0,9)) as f
  order by a,b,c,d,e,f;
create index tmp1mil__c on tmp1mil(c);
vacuum analyze tmp1mil;
select * from pg_stats where tablename='tmp1mil';

\timing
explain select count(*) from tmp1mil where c5;
select count(*) from tmp1mil where c5;
select count(*) from tmp1mil where c5; -- 615 ms seqscan

set enable_seqscan = false;
explain select count(*) from tmp1mil where c5;
select count(*) from tmp1mil where c5;
select count(*) from tmp1mil where c5; -- 425 ms bitmapscan

set enable_bitmapscan to false;
explain select count(*) from tmp1mil where c5;
select count(*) from tmp1mil where c5;
select count(*) from tmp1mil where c5; -- 342 ms indexscan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] new correlation metric

2008-10-27 Thread Ron Mayer


Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

...bitmap cost estimates didn't also change much


By definition, a bitmap scan's cost isn't affected by index order
correlation.


No?   I think I understand that for index scans the correlation
influenced how many data pages are estimated to get sucked in.

Wouldn't a bitmap scan using a single index also fetch roughly
the same number of data pages as an index scan?

I'm not complaining, since 8.3's doing great on all my real-world
queries.   And sorry for my naive questions - feel free to tell
me to just read the code if this is something I should be able
to figure out myself.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Cross-column statistics revisited

2008-10-17 Thread Ron Mayer


Tom Lane wrote:

A bad estimate for physical-position correlation has only limited
impact,


Ah!  This seems very true with 8.3 but much less true with 8.0.

On a legacy 8.0 system I have a hard time avoiding cases where
a query like
 select * from addresses where add_state_or_province = 'CA';
does a 2-second full-table scan instead of a 300ms index scan
thanks to a poor physical order guess.

I just sucked the table into 8.3 and am pleased to say that
it picks a 200ms bitmap scan even with the misleading correlation.

Thanks for bitmap scans guys!

I'll shut up about this physical ordering stuff now
and try to do better upgrading before posting.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Cross-column statistics revisited

2008-10-16 Thread Ron Mayer


Robert Haas wrote:

I think the real question is: what other kinds of correlation might
people be interested in representing?


Yes, or to phrase that another way: What kinds of queries are being
poorly optimized now and why?


The one that affects our largest tables are ones where we
have an address (or other geo-data) clustered by zip, but
with other columns (city, county, state, school-zone, police
beat, etc) used in queries.

Postgres considers those unclustered (correlation 0 in the stats),
despite all rows for a given value residing on the same few pages.


I could imagine that this could be handled by either some cross-column
correlation (each zip has only 1-2 cities); or by an enhanced
single-column statistic (even though cities aren't sorted alphabetically,
all rows on a page tend to refer to the same city).



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Cross-column statistics revisited

2008-10-16 Thread Ron Mayer


Josh Berkus wrote:

Yes, or to phrase that another way: What kinds of queries are being
poorly optimized now and why?


Well, we have two different correlation problems.  One is the problem of 
dependant correlation, such as the 1.0 correlation of ZIP and CITY fields 
as a common problem.  This could in fact be fixed, I believe, via a linear 
math calculation based on the sampled level of correlation, assuming we 
have enough samples.  And it's really only an issue if the correlation is 

0.5.


I'd note that this can be an issue even without 2 columns involved.

I've seen a number of tables where the data is loaded in batches
so similar-values from a batch tend to be packed into relatively few pages.

Thinks a database for a retailer that nightly aggregates data from
each of many stores.  Each incoming batch inserts the store's data
into tightly packed disk pages where most all rows on the page are for
that store.   But those pages are interspersed with pages from other
stores.

I think I like the ideas Greg Stark had a couple years ago:
http://archives.postgresql.org/pgsql-hackers/2006-09/msg01040.php
   ...sort the sampled values by value
   and count up the average number of distinct blocks per value Or
   perhaps we need a second histogram where the quantities are of
   distinct pages rather than total records We might also need a
   separate average number of n-block spans per value
since those seem to me to lead more directly to values like blocks
that need to be read.



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] The Axe list

2008-10-11 Thread Ron Mayer


[EMAIL PROTECTED] wrote:

So it seems that intagg should rather live in a section examples than
in contrib?


Perhaps.   Seems my old intagg use case from 8.1 is not really needed
anymore since it seems ANY got much smarter since then.   Cool.




=
== With 8.1
=
fli=# explain analyze
fli-#   select * from lines, selected_lines
fli-#   where selected_lines.id = 16238 and tlid = ANY (line_ids);
   QUERY PLAN

 Nested Loop  (cost=24.51..16899784.67 rows=166031616 width=202) (actual 
time=0.980..615547.605 rows=2 loops=1)
   Join Filter: (outer.tlid = ANY (inner.line_ids))
   -  Seq Scan on lines  (cost=0.00..1956914.72 rows=55343872 width=166) 
(actual time=0.012..160106.897 rows=55291697 loops=1)
   -  Materialize  (cost=24.51..24.57 rows=6 width=36) (actual 
time=0.002..0.003 rows=1 loops=55291697)
 -  Seq Scan on selected_lines  (cost=0.00..24.50 rows=6 width=36) 
(actual time=0.012..0.016 rows=1 loops=1)
   Filter: (id = 16238)
 Total runtime: 615547.680 ms
(7 rows)


fli=# explain analyze
fli-#   select *
fli-#  from lines,
fli-#  (select int_array_enum(line_ids) as lid from selected_lines where 
id=16238) as z
fli-#   where lid = lines.tlid;
QUERY PLAN
--
 Nested Loop  (cost=0.00..54.42 rows=6 width=170) (actual time=36.755..42.268 
rows=2 loops=1)
   -  Seq Scan on selected_lines  (cost=0.00..24.52 rows=6 width=32) (actual 
time=0.023..0.029 rows=2 loops=1)
 Filter: (id = 16238)
   -  Index Scan using rtgr_lines__tlid on lines  (cost=0.00..4.96 rows=1 
width=166) (actual time=21.105..21.108 rows=1 loops=2)
 Index Cond: (outer.?column1? = lines.tlid)
 Total runtime: 42.352 ms
(6 rows)

=
==  With HEAD
=

fli=# explain analyze
  select * from lines, selected_lines
  where selected_lines.id = 16238 and tlid = ANY (line_ids);

fli-# fli-#QUERY 
PLAN
-
 Nested Loop  (cost=36.44..424.78 rows=61 width=210) (actual time=0.335..0.421 
rows=2 loops=1)
   -  Seq Scan on selected_lines  (cost=0.00..24.50 rows=6 width=36) (actual 
time=0.018..0.021 rows=1 loops=1)
 Filter: (id = 16238)
   -  Bitmap Heap Scan on lines  (cost=36.44..66.49 rows=10 width=174) (actual 
time=0.293..0.374 rows=2 loops=1)
 Recheck Cond: (lines.tlid = ANY (selected_lines.line_ids))
 -  Bitmap Index Scan on rtgr_lines__tlid  (cost=0.00..36.44 rows=10 
width=0) (actual time=0.251..0.251 rows=2 loops=1)
   Index Cond: (lines.tlid = ANY (selected_lines.line_ids))
 Total runtime: 0.653 ms
(8 rows)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] The Axe list

2008-10-11 Thread Ron Mayer


Josh Berkus wrote:
So it sounds like intagg is still in use/development.  But ... is it 
more of an example, or is it useful as a type/function in production?


Where I work we (and our customers) use it in our production systems.

At first glance it seems our reasons for using it are mostly
legacy reasons dating to 8.1 where intagg was the best way to
write some queries.  At least some of these seem to be unnecessary
with 8.3.  If intagg's at risk of going away soon I could
further check the range of queries where we use it against 8.3
or CVS head if that's useful to the discussion.

From our testing notes, here's another 8.1 query where we had
order-of-magnitude speedups using intagg and friends.
-- with 3
-- explain analyze select fac_nam  from userfeatures.point_features  join 
entity_facets using (entity_id)  where featureid=115 group by fac_nam;
-- -- Total runtime: 7125.322 ms
-- select fac_nam from (select distinct int_array_enum(fac_ids) as fac_id from 
(select distinct fac_ids from entity_facids natural join point_features where 
featureid=115) as a) as a join facet_lookup using (fac_id);
-- -- Total runtime: 1297.558 ms
-- explain analyze select fac_nam from (select int_array_enum(fac_ids) as 
fac_id from (select fac_ids from entity_facids natural join point_features 
where featureid=115 group by fac_ids) as a group by int_array_enum(fac_ids)) as 
a join facet_lookup using (fac_id) order by fac_nam;
-- -- Total runtime: 1164.258 ms
-- explain analyze select fac_nam from (select distinct int_array_enum(fac_ids) 
as fac_id from (select intarray_union_agg(fac_ids) as fac_ids from 
entity_facids natural join point_features where featureid=115) as a) as a join 
facet_lookup using (fac_id);
-- -- Total runtime: 803.187 ms
I can check it on 8.3 monday.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] How is random_page_cost=4 ok?

2008-10-10 Thread Ron Mayer


Tom Lane wrote:

In particular, if the OS lays out successive file pages in a way that
provides zero latency between logically adjacent blocks, I'd bet a good
bit that a Postgres seqscan would miss the read timing every time, and
degrade to handling about one block per disk rotation.


Unless the OS does some readahead when it sees something like a seq scan?


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] The Axe list

2008-10-10 Thread Ron Mayer


Josh Berkus wrote:

intagg: ...  Has not been updated since 2001.


Really?  Just a couple years ago (2005) bugs we reported were
still getting fixed in it:
http://archives.postgresql.org/pgsql-bugs/2005-03/msg00202.php
http://archives.postgresql.org/pgsql-bugs/2005-04/msg00165.php

Here's one use-case I had at the time.
http://archives.postgresql.org/pgsql-general/2005-04/msg01249.php
I think we still use that code, but I suppose I can re-write it
to use features that were added since then.
And hey - where'd README.int_aggregate go? IIRC that had
a couple interesting use-cases too.

I also like intagg, because it's kinda like a hello world for
writing one kind of C extensions.   I'm not saying it needs to
stay in contrib.  Perhaps it could live on as an example in the docs?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-10-09 Thread Ron Mayer


Kevin Grittner wrote:
Kevin Grittner [EMAIL PROTECTED] wrote: 
 
Even more surprising is the behavior for interval(1) here:

[ some context with nonsurprising examples removed ...]
ccdev=# select '1 year 2 mons 3 days 04:05:06.64321'::interval(1);
 interval
--
 1 year 2 mons 3 days 04:05:06.60
(1 row)

That trailing zero should be considered a bug.


Is there a consensus that we don't want that trailing zero?
I notice that datetime.c's TrimTrailingZeros(char *str) has
the comment:
/* chop off trailing zeros... but leave at least 2 fractional digits */
that suggests that the trailing zero was intentional, but I
can't find any reasons why 2 fractional disgits were left.

The same function's also used for timestamps, so if we remove that
trailing zero in both places we'll see some regression differences
where we get
! | Mon Feb 10 17:32:01.5 1997 PST |1997 |7 |   1
instead of
! | Mon Feb 10 17:32:01.50 1997 PST |1997 |7 |   1

IMHO we don't want the extra zero for timestamps either.


If people agree I'll fold it into the patch dealing with
the other interval rounding eccentricities I have.

Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

[some other interval rounding example]


I don't much like the forced rounding to two digits here, but changing
that doesn't seem like material for back-patching.  Are you going to
fix that up while working on your other patches?





--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-10-09 Thread Ron Mayer


Tom Lane wrote:

In the integer-timestamp world we know that the number is exact in
microseconds.  We clearly ought to be prepared to display up to six
fractional digits, but suppressing trailing zeroes in that seems
appropriate.


Great.


We could try to do the same in the float case, but I'm a bit worried
about finding ourselves showing 1234567.79 where it should be
1234567.8.


If I understand the code right fsec should mostly be values
between -1 and 1 anyway, because even in the floating point
case seconds are carried in the tm-tm_sec part.
It looks to me that a double should be plenty to do
microseconds so long as we don't put big numbers into fsec.
  printf(%.99f\n,59.11);
  59.11426907882 ...

Anyway - I'll try showing up-to-6-digits in both cases and
seeing if I can find a test case that breaks.

I guess this rounding trickiness with rounding explains some of
the bizarre code like this too:
   #if 0
/* chop off trailing one to cope with interval rounding */
if (strcmp(str + len - 4, 0001) == 0)
{
len -= 4;
*(str + len) = '\0';
}
   #endif

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-10-09 Thread Ron Mayer


Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

Tom Lane wrote:

We could try to do the same in the float case, but I'm a bit worried
about finding ourselves showing 1234567.79 ...

If I understand the code right [I didn't...]


The problem is ... seconds field that includes hours,
minutes, seconds, and fractional seconds...Here's an example...
regression=# select '1234567890 hours 0.123 sec'::interval;
... 1234567890:00:00.123047


Hmm.  That's also an existence proof that we're not too concerned
about showing 6 imprecise digits anyway (at least for some 8.3
DateStyles).  Doesn't seem like it'd hurt too much if we show
them for all the IntervalStyles.


Since there's a (somewhat arbitrary) limitation of the hours to 2^31,
this is close to the worst possible case.  (Hm, maybe someone actually
did the math and decided that 2 fractional digits ...


Or I guess we could truncate to 2 digits only in the float case;
or truncate to 2 digits only if we're using the float case and
have large values.   But that extra complexity doesn't seem
worth it to me - especially since it seems to only affect
people who do two non-default things (pick a date/interval style
that used to truncate to 2, and --disable-integer-datetimes).

I put a patch up at http://0ape.com/postgres_interval_patches
that does what I think seems getting reasonable.   For better
or worse, it depends on the other two interval patches I was
working on, but I could make a version that doesn't depend on
those as well if people prefer that.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Interval output bug in HAVE_INT64_TIMESTAMP


Seems to me there's a bug in HEAD (and probably old branches
as well) when compiled with HAVE_INT64_TIMESTAMP.  As shown below
It sometimes shows things like -6.-70 secs where 8.3
showed -6.70 secs.

I think the attached one-liner patch fixes this, as well as
another roundoff regression between HEAD and 8.3.

  Ron Mayer


== ON HEAD

regression=#  set datestyle to sql;
SET
regression=#  select '-10 mons -3 days +03:55:06.70'::interval;
  interval

 @ 10 mons 3 days -3 hours -55 mins -6.-70 secs ago
(1 row)

regression=# select '1 year 2 mons 3 days 04:05:06.69'::interval;
interval
-
 @ 1 year 2 mons 3 days 4 hours 5 mins 6.69 secs
(1 row)


== ON 8.3

pg83=#  set datestyle to sql;
SET
pg83=# select '-10 mons -3 days +03:55:06.70'::interval;
 interval
---
 @ 10 mons 3 days -3 hours -55 mins -6.70 secs ago
(1 row)


pg83=#  select '1 year 2 mons 3 days 04:05:06.69'::interval;
interval
-
 @ 1 year 2 mons 3 days 4 hours 5 mins 6.70 secs
(1 row)
*** a/src/backend/utils/adt/datetime.c
--- b/src/backend/utils/adt/datetime.c
***
*** 3791,3797  EncodeInterval(struct pg_tm * tm, fsec_t fsec, int style, char *str)
  	is_before = TRUE;
  }
  sprintf(cp, %s%d.%02d secs, is_nonzero ?   : ,
! 		tm-tm_sec, ((int) sec) / 1);
  cp += strlen(cp);
  #else
  fsec += tm-tm_sec;
--- 3791,3797 
  	is_before = TRUE;
  }
  sprintf(cp, %s%d.%02d secs, is_nonzero ?   : ,
! 		tm-tm_sec, (int) abs(rint(sec / 1.0)));
  cp += strlen(cp);
  #else
  fsec += tm-tm_sec;

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle


Ron Mayer wrote:

Ron Mayer wrote:

Tom Lane wrote:
...GUC that selected PG traditional, SQL-standard... interval output 
format seems like it could be a good idea.

This is an update to the earlier SQL-standard-interval-literal output
patch that I submitted here:
http://archives.postgresql.org/message-id/[EMAIL PROTECTED] 


Yet another update - mostly bringing the patch current with HEAD
now that the previous commit fest is over; and also posting it again
since I have a followup patch (for ISO 8601 interval input and output)
that is based on this one and I want the patches lines-of-code to match.

  Ron
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***
*** 4016,4021  SET XML OPTION { DOCUMENT | CONTENT };
--- 4016,4043 
/listitem
   /varlistentry
  
+  varlistentry id=guc-intervalstyle xreflabel=IntervalStyle
+   termvarnameIntervalStyle/varname (typestring/type)/term
+   indexterm
+primaryvarnameIntervalStyle/ configuration parameter/primary
+   /indexterm
+   listitem
+para
+ Sets the display format for interval values. 
+ The value literalsql_standard/ will output SQL Standard
+ strings when given intervals that conform to the SQL
+ standard (either year-month only or date-time only; and no
+ mixing of positive and negative components).
+ The value literalpostgres/ will output intervals in
+ a format that matches what old releases had output when
+ the DateStyle was set to literal'ISO'/.
+ The value literalpostgres_verbose/ will output intervals in
+ a format that matches what old releases had output when
+ the DateStyle was set to literal'SQL'/.
+/para
+   /listitem
+  /varlistentry
+ 
   varlistentry id=guc-timezone xreflabel=timezone
termvarnametimezone/varname (typestring/type)/term
indexterm
*** a/doc/src/sgml/datatype.sgml
--- b/doc/src/sgml/datatype.sgml
***
*** 2213,2218  January 8 04:05:06 1999 PST
--- 2213,2305 
  /para
 /sect2
  
+sect2 id=interval-output
+ titleInterval Output/title
+ 
+ indexterm
+  primaryinterval/primary
+  secondaryoutput format/secondary
+  seealsoformatting/seealso
+ /indexterm
+ 
+ para
+  The output format of the interval types can be set to one of the four
+  styles literalsql_standard/, 
+  literalpostgres/, or literalpostgres_verbose/.The default
+  is the literalpostgres/ format.  
+  xref
+  linkend=interval-style-output-table shows examples of each
+  output style.
+ /para
+ 
+ para
+  The literalsql_standard/ style will output SQL standard
+  interval literal strings where the value of the interval
+  value consists of only a year-month component or a datetime
+  component (as required by the sql standard).   For an interval
+  containing both a year-month and a datetime component, the
+  output will be a SQL Standard unquoted year-month literal
+  string joined to a SQL Standard unquoted datetime literal
+  string with a space in between.
+ /para
+ 
+ para
+  The literalpostgres/ style will output intervals that match
+  the style PostgreSQL 8.3 outputed when the xref linkend=guc-datestyle
+  parameter was set to literalISO/.
+ /para
+ 
+ para
+  The literalpostgres_verbose/ style will output intervals that match
+  the style PostgreSQL 8.3 outputed when the xref linkend=guc-datestyle
+  parameter was set to literalSQL/.
+ /para
+ 
+  table id=interval-style-output-table
+ 	   titleInterval Style Example/title
+ 	   tgroup cols=2
+ 		thead
+ 		 row
+ 		  entryStyle Specification/entry
+ 		  entryYear-Month Interval/entry
+ 		  entryDateTime Interval/entry
+ 		  entryNonstandardrd Extended Interval/entry
+ 		 /row
+ 		/thead
+ 		tbody
+ 		 row
+ 		  entrysql_standard/entry
+ 		  entry1-2/entry
+ 		  entry3 4:05:06/entry
+ 		  entry-1-2 +3 -4:05:06/entry
+ 		 /row
+ 		 row
+ 		  entrypostgres/entry
+ 		  entry1 year 2 mons/entry
+ 		  entry3 days 04:05:06/entry
+ 		  entry -1 years -2 mons +3 days -04:05:06/entry
+ 		 /row
+ 		 row
+ 		  entrypostgres_verbose/entry
+ 		  entry@ 1 year 2 mons/entry
+ 		  entry@ 3 days 4 hours 5 mins 6 secs/entry
+ 		  entry@ 1 year 2 mons -3 days 4 hours 5 mins 6 secs ago/entry
+ 		 /row
+ 		/tbody
+ 	 /tgroup
+ 	/table
+ 
+  para
+  Note that literalsql_standard/ style will only produce strictly 
+  standards-conforming string sliterals when given a strictly SQL-standard interval
+  value - meaning that it needs to be a pure year-month or datetime
+  interval and not mix positive and negative components.
+  /para
+ 
+/sect2
+ 
+ 
+ 
 sect2 id=datatype-timezones
  titleTime Zones/title
  
*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c
***
*** 229,234

[HACKERS] Patch for ISO-8601-Interval Input and output.


Ron Mayer wrote:

Tom Lane wrote:

In fact, given that we are now
somewhat SQL-compliant on interval input, a GUC that selected
PG traditional, SQL-standard, or ISO 8601 interval output format seems
like it could be a good idea.


This patch (that works on top of the IntervalStyle patch I
posted earlier today) adds support for ISO8601 standard[0]
Time Interval Durations of the format with designators
(section 4.4.4.2.1).   The other ISO 8601 types of intervals
deal with start and end points, so this one seemed most relevant.

It builds on a patch I had earlier submitted back in 2003[1],
where people noted that we wanted sql-standard intervals
first; but I see that ISO 8601 intervals did make it to the
todo list.

I updated the docs, but I still need to add regression tests,
so it's still a WIP, but I think the code's ready (I've been
using an older version of this patch internally since '03) so
I'd appreciate feedback.

[0] 
http://isotc.iso.org/livelink/livelink/4021199/ISO_8601_2004_E.zip?func=doc.Fetchnodeid=4021199
[1] http://archives.postgresql.org/pgsql-patches/2003-09/msg00121.php
[2] http://wiki.postgresql.org/wiki/Todo


*** a/doc/src/sgml/datatype.sgml
--- b/doc/src/sgml/datatype.sgml
***
*** 1975,1980  January 8 04:05:06 1999 PST
--- 1975,1996 
   /para
  
   para
+   Alternatively, typeinterval/type values can be written as 
+   ISO 8601 time intervals, using the Format with time-unit designators,
+   or PnYnMnDTnHnMnS.   This format always starts with the character 
+   literal'P'/, followed by a string of values followed by single
+   character time-unit designators.  A literal'T'/ separates the 
+   date and time parts of the interval.
+   In this format, literal'n'/ gets replaced by a number, and 
+literalY/ represents years, 
+literalM/ (in the date part) months,
+literalD/ months,
+literalH/ hours,
+literalM/ (in the time part) minutes,
+and literalS/ seconds.
+  /para
+ 
+  para
Internally typeinterval/ values are stored as months, days,
and seconds. This is done because the number of days in a month
varies, and a day can have 23 or 25 hours if a daylight savings
***
*** 2224,2231  January 8 04:05:06 1999 PST
  
  para
   The output format of the interval types can be set to one of the four
!  styles literalsql_standard/, 
!  literalpostgres/, or literalpostgres_verbose/.The default
   is the literalpostgres/ format.  
   xref
   linkend=interval-style-output-table shows examples of each
--- 2240,2247 
  
  para
   The output format of the interval types can be set to one of the four
!  styles literalsql_standard/, literaliso_8601/, 
!  literalpostgres/, or literalpostgres_verbose/.  The default
   is the literalpostgres/ format.  
   xref
   linkend=interval-style-output-table shows examples of each
***
*** 2244,2249  January 8 04:05:06 1999 PST
--- 2260,2281 
  /para
  
  para
+  The literaliso_8601/ style will output ISO 8601 
+  time intervals using the format with time-unit designators
+  This format always starts with the character 
+   literal'P'/, followed by a string of values followed by single
+   character time-unit designators.  A literal'T'/ separates the 
+   date and time parts of the interval.
+   In this format, literal'n'/ gets replaced by a number, and 
+literalY/ represents years, 
+literalM/ (in the date part) months,
+literalD/ months,
+literalH/ hours,
+literalM/ (in the time part) minutes,
+and literalS/ seconds.
+ /para
+ 
+ para
   The literalpostgres/ style will output intervals that match
   the style PostgreSQL 8.3 outputed when the xref linkend=guc-datestyle
   parameter was set to literalISO/.
***
*** 2274,2283  January 8 04:05:06 1999 PST
  		  entry-1-2 +3 -4:05:06/entry
  		 /row
  		 row
  		  entrypostgres/entry
  		  entry1 year 2 mons/entry
  		  entry3 days 04:05:06/entry
! 		  entry -1 years -2 mons +3 days -04:05:06/entry
  		 /row
  		 row
  		  entrypostgres_verbose/entry
--- 2306,2321 
  		  entry-1-2 +3 -4:05:06/entry
  		 /row
  		 row
+ 		  entryiso_8601/entry
+ 		  entryP1Y2M/entry
+ 		  entryP3DT4H5M6/entry
+ 		  entryP-1Y-2M3DT-4H-5M-6/entry
+ 		 /row
+ 		 row
  		  entrypostgres/entry
  		  entry1 year 2 mons/entry
  		  entry3 days 04:05:06/entry
! 		  entry-1 year -2 mons 3 days -04:05:06/entry
  		 /row
  		 row
  		  entrypostgres_verbose/entry
***
*** 2293,2299  January 8 04:05:06 1999 PST
   Note that literalsql_standard/ style will only produce strictly 
   standards-conforming string sliterals when given a strictly SQL-standard interval
   value - meaning that it needs to be a pure year-month or datetime
!  interval and not mix positive

Re: [HACKERS] Interval output bug in HAVE_INT64_TIMESTAMP


Tom Lane wrote:

Yeah, bug all the way back --- applied.
I don't much like the forced rounding to two digits here, but changing
that doesn't seem like material for back-patching.  Are you going to
fix that up while working on your other patches?


Gladly.  I hate that too.

I think I can also re-factor some of the copy-paste there
to remove about 200 lines of code duplicated between the interval
styles.   I'll get this one in there as well, and try posting
a cleanup patch tonight.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Interval output bug in HAVE_INT64_TIMESTAMP


Ron Mayer wrote:

Tom Lane wrote:

Yeah, bug all the way back --- applied.
I don't much like the forced rounding to two digits here, but changing
that doesn't seem like material for back-patching.  Are you going to
fix that up while working on your other patches?


Gladly.  I hate that too.


Attached is a WIP interval code-cleanup patch that tries to reuse
more code between interval style.   Once side-benefit is that it
makes rounding more consistent (so we should have fewer bugs like
that last rounding one that only affected one date style), as well
as removing roughly 250 lines of near-copy-and-paste code (some
of it my doing in my previous patches) and IMHO making the code
easier to read.

This particular patch lives on top of the other 2 I submitted,
but I could make a similar one that doesn't depend on those
if either of those get rejected - though the savings wouldn't
be as great since some of the benefit is that all 4 datestyles
can share some of the refactored code.  It does do some
extra function calls; but I think any cost there's easily
offset by the removal of gratuitous (AFAICT) strlen()'s that
were scattered in the existing code and are now removed.

Despite the rounding differences it seems to have no effect
on the regression test output - presumably because these rounding
differences are more obscure corner cases.

I consider it a WIP for 3 reasons:

 1) Does anyone object to the not-quite-backward compatible
rounding (forced 2 digits; now some rounding instead
of truncation).
 2) Do we even want broad scale refactoring like this, or do
people prefer minimal changes (like my previous patches)
that tried to as much of the old code intact preferred.
 3) If people like this kind of refactoring, there's more
I could do - especially if I can I find a place to put
functions the ECPG stuff can call too.


*** a/src/backend/utils/adt/datetime.c
--- b/src/backend/utils/adt/datetime.c
***
*** 406,421  static void
  TrimTrailingZeros(char *str)
  {
  	int			len = strlen(str);
- 
- #if 0
- 	/* chop off trailing one to cope with interval rounding */
- 	if (strcmp(str + len - 4, 0001) == 0)
- 	{
- 		len -= 4;
- 		*(str + len) = '\0';
- 	}
- #endif
- 
  	/* chop off trailing zeros... but leave at least 2 fractional digits */
  	while (*(str + len - 1) == '0'  *(str + len - 3) != '.')
  	{
--- 406,411 
***
*** 2724,2732  DecodeSpecial(int field, char *lowtoken, int *val)
  
  
  /*
!  * A small helper function to avoid cutpaste code in DecodeIso8601Interval
   */
! static void adjust_fval(double fval,struct pg_tm * tm, fsec_t *fsec, int scale)
  {
  	int	sec;
  	if (fval == 0) return;
--- 2714,2723 
  
  
  /*
!  * Small helper functions to avoid cutpaste code in DecodeInterval and DecodeIso8601Interval
   */
! static void 
! adjust_fractional_seconds(double fval,struct pg_tm * tm, fsec_t *fsec, int scale)
  {
  	int	sec;
  	if (fval == 0) return;
***
*** 2734,2745  static void adjust_fval(double fval,struct pg_tm * tm, fsec_t *fsec, int scale)
  	sec		= fval;
  	tm-tm_sec += sec;
  #ifdef HAVE_INT64_TIMESTAMP
! 	*fsec	   += ((fval - sec) * 100);
  #else
  	*fsec	   += (fval - sec);
  #endif
  }
  
  
  /* DecodeISO8601Interval()
   *
--- 2725,2748 
  	sec		= fval;
  	tm-tm_sec += sec;
  #ifdef HAVE_INT64_TIMESTAMP
! 	*fsec	   += rint((fval - sec) * 100);
  #else
  	*fsec	   += (fval - sec);
  #endif
  }
  
+ static void 
+ adjust_fractional_days(double fval,struct pg_tm * tm, fsec_t *fsec, int scale)
+ {
+ 	int	extra_days;
+ 	if (fval == 0) return;
+ 	fval *= scale;
+ 	extra_days = fval;
+ 	tm-tm_mday += extra_days;
+ 	fval -= extra_days;
+ 	adjust_fractional_seconds(fval,tm,fsec, SECS_PER_DAY);
+ }
+ 
  
  /* DecodeISO8601Interval()
   *
***
*** 2768,2775  int
  DecodeISO8601Interval(char *str, struct pg_tm * tm, fsec_t *fsec) 
  {
  	charunit;
- 	int		fmask = 0,
- 			tmask;
  	int		val;
  	double	fval;
  	int		datepart = true;
--- 2771,2776 
***
*** 2785,2798  DecodeISO8601Interval(char *str, struct pg_tm * tm, fsec_t *fsec)
  
  	/*
  	 * An ISO 8601 time-interval by duration only must start
! 	 * with a 'P'.  If it contains a date-part, 'p' will be the
! 	 * only character in the field.  If it contains no date part
! 	 * it will contain exactly to characters 'PT' indicating a
! 	 * time part.
! 	 * Anything else does not match an ISO 8601 basic interval
! 	 * and will be treated like a traditional postgresql interval.
  	 */
! 	if (!(str[0] == 'P'))
  	{
  		return DTERR_BAD_FORMAT;
  	}
--- 2786,2795 
  
  	/*
  	 * An ISO 8601 time-interval by duration only must start
! 	 * with a 'P' and needs to be at least 3 characters long to
! 	 * insure that it had a field set.
  	 */
! 	if (strlen(str)3 || !(str[0] == 'P'))
  	{
  		return DTERR_BAD_FORMAT;
  	}
***
*** 2826,2860  DecodeISO8601Interval(char *str, struct pg_tm * tm, fsec_t *fsec

Re: [HACKERS] PostgreSQL future ideas

2008-09-23 Thread Ron Mayer


Gevik Babakhani wrote:

Has there been any idea to port PG to a more modern programming language
like C++? Of course there are some minor obstacles like a new OO design,
this being a gigantic task to perform and rewriting almost everything etc...
I am very interested to hear your opinion.


Gevik, of course you're free to fork the project and try this yourself.

I'd caution you that neither OO nor C++ are particularly modern
(Stroustrup's objects-on-C work dates back to the 1970's).  And that
of the OO languages, C++ is one of the worst in terms of OO capabilities.

If your theory favoring a modern language is thinking that this'll
give you efficiencies (either in development time or runtime), you
might consider Erlang instead.  It's Functional and Concurrency and
Fault Tolerance oriented features would IMHO be more useful for large
reliable servers than anything C++ has to offer.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Initial prefetch performance testing

2008-09-22 Thread Ron Mayer


Gregory Stark wrote:

Simon Riggs [EMAIL PROTECTED] writes:

I'm not in favour of introducing the concept of spindles


In principle I quite strongly disagree with this
Number of blocks to prefetch is an internal implementation detail that the DBA
has absolutely no way to know what the correct value is. 


Even more often on systems I see these days, spindles
is an implementation detail that the DBA has no way to know
what the correct value is.

For example, on our sites hosted with Amazon's compute cloud (a great
place to host web sites), I know nothing about spindles, but know
about Amazon Elastic Block Store[2]'s and Instance Store's[1].   I
have some specs and are able to run benchmarks on them; but couldn't
guess how many spindles my X% of the N-disk device that corresponds
to.  For another example, some of our salesguys with SSD drives
have 0 spindles on their demo machines.

I'd rather a parameter that expressed things more in terms of
measurable quantities -- perhaps seeks/second?  perhaps
random-access/sequential-access times?



[1] http://www.amazon.com/gp/browse.html?node=201590011
[2] 
http://www.amazon.com/b/ref=sc_fe_c_0_201590011_1?ie=UTF8node=689343011no=201590011me=A36L942TSJ2AJA


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Interval literal rounding bug(?) and patch.

2008-09-22 Thread Ron Mayer


I think it's a bug that these 3 different ways of writing 0.7 seconds
produce different results from each other on HEAD.

head=# select interval '0:0:0.7', interval '@ 0.70 secs', interval '0.7 
seconds';
  interval   |interval |interval
-+-+-
 00:00:00.70 | 00:00:00.69 | 00:00:00.69
(1 row)

The attached patch will make all of those output 00:00:00.70 which.

Postgres 8.3 tended to output the 00:00:00.70 like this patch, I believe
because it didn't default to HAVE_INT64_TIMESTAMP like HEAD is.   The patch
seems to pass the existing regression tests.

Does this seem reasonable?

   Ron


*** a/src/backend/utils/adt/datetime.c
--- b/src/backend/utils/adt/datetime.c
***
*** 2888,2894  DecodeInterval(char **field, int *ftype, int nf, int range,
  {
  	case DTK_MICROSEC:
  #ifdef HAVE_INT64_TIMESTAMP
! 		*fsec += val + fval;
  #else
  		*fsec += (val + fval) * 1e-6;
  #endif
--- 2888,2894 
  {
  	case DTK_MICROSEC:
  #ifdef HAVE_INT64_TIMESTAMP
! 		*fsec += rint(val + fval);
  #else
  		*fsec += (val + fval) * 1e-6;
  #endif
***
*** 2897,2903  DecodeInterval(char **field, int *ftype, int nf, int range,
  
  	case DTK_MILLISEC:
  #ifdef HAVE_INT64_TIMESTAMP
! 		*fsec += (val + fval) * 1000;
  #else
  		*fsec += (val + fval) * 1e-3;
  #endif
--- 2897,2903 
  
  	case DTK_MILLISEC:
  #ifdef HAVE_INT64_TIMESTAMP
! 		*fsec += rint((val + fval) * 1000);
  #else
  		*fsec += (val + fval) * 1e-3;
  #endif
***
*** 2907,2913  DecodeInterval(char **field, int *ftype, int nf, int range,
  	case DTK_SECOND:
  		tm-tm_sec += val;
  #ifdef HAVE_INT64_TIMESTAMP
! 		*fsec += fval * 100;
  #else
  		*fsec += fval;
  #endif
--- 2907,2913 
  	case DTK_SECOND:
  		tm-tm_sec += val;
  #ifdef HAVE_INT64_TIMESTAMP
! 		*fsec += rint(fval * 100);
  #else
  		*fsec += fval;
  #endif
***
*** 2932,2938  DecodeInterval(char **field, int *ftype, int nf, int range,
  			sec = fval;
  			tm-tm_sec += sec;
  #ifdef HAVE_INT64_TIMESTAMP
! 			*fsec += (fval - sec) * 100;
  #else
  			*fsec += fval - sec;
  #endif
--- 2932,2938 
  			sec = fval;
  			tm-tm_sec += sec;
  #ifdef HAVE_INT64_TIMESTAMP
! 			*fsec += rint((fval - sec) * 100);
  #else
  			*fsec += fval - sec;
  #endif
***
*** 2950,2956  DecodeInterval(char **field, int *ftype, int nf, int range,
  			sec = fval;
  			tm-tm_sec += sec;
  #ifdef HAVE_INT64_TIMESTAMP
! 			*fsec += (fval - sec) * 100;
  #else
  			*fsec += fval - sec;
  #endif
--- 2950,2956 
  			sec = fval;
  			tm-tm_sec += sec;
  #ifdef HAVE_INT64_TIMESTAMP
! 			*fsec += rint((fval - sec) * 100);
  #else
  			*fsec += fval - sec;
  #endif
***
*** 2969,2975  DecodeInterval(char **field, int *ftype, int nf, int range,
  			sec = fval;
  			tm-tm_sec += sec;
  #ifdef HAVE_INT64_TIMESTAMP
! 			*fsec += (fval - sec) * 100;
  #else
  			*fsec += fval - sec;
  #endif
--- 2969,2975 
  			sec = fval;
  			tm-tm_sec += sec;
  #ifdef HAVE_INT64_TIMESTAMP
! 			*fsec += rint((fval - sec) * 100);
  #else
  			*fsec += fval - sec;
  #endif
***
*** 2995,3001  DecodeInterval(char **field, int *ftype, int nf, int range,
  sec = fval;
  tm-tm_sec += sec;
  #ifdef HAVE_INT64_TIMESTAMP
! *fsec += (fval - sec) * 100;
  #else
  *fsec += fval - sec;
  #endif
--- 2995,3001 
  sec = fval;
  tm-tm_sec += sec;
  #ifdef HAVE_INT64_TIMESTAMP
! *fsec += rint((fval - sec) * 100);
  #else
  *fsec += fval - sec;
  #endif
***
*** 3022,3028  DecodeInterval(char **field, int *ftype, int nf, int range,
  sec = fval;
  tm-tm_sec += sec;
  #ifdef HAVE_INT64_TIMESTAMP
! *fsec += (fval - sec) * 100;
  #else
  *fsec += fval - sec;
  #endif
--- 3022,3028 
  sec = fval;
  tm-tm_sec += sec;
  #ifdef HAVE_INT64_TIMESTAMP
! *fsec += rint((fval - sec) * 100);
  #else
  *fsec += fval - sec;
  #endif

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Initial prefetch performance testing

2008-09-22 Thread Ron Mayer


Gregory Stark wrote:

Ron Mayer [EMAIL PROTECTED] writes:

I'd rather a parameter that expressed things more in terms of
measurable quantities [...]


...What we're
dealing with now is an entirely orthogonal property of your system: how many
concurrent requests can the system handle.


Really?  I'd have thought you'd want to give the OS enough guesses
about the future that it's elevator algorithms for the drive heads
don't keep seeking back-and-forth but rather do as much per sweep
across a device that they can.


Ironically I'm pretty happy to lose this argument because EDB is interested in
rolling this into its dynamic tuning module. If there's a consensus -- by my
count three people have spoken up already which is more than usual -- then
I'll gladly concede. Anyone object to going back to preread_pages? Or should
it be prefetch_pages? prefetch_blocks? Something else?


Well - as you pointed out, I'm not on their side of the debate either.
I'm not sure what a relevant measurable parameter would be so I'm not
being too helpful in the conversation either.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle

2008-09-20 Thread Ron Mayer


Ron Mayer wrote:

Tom Lane wrote:
...GUC that selected PG traditional, SQL-standard... interval output 
format seems like it could be a good idea.




This is an update to the earlier SQL-standard-interval-literal output
patch that I submitted here:
http://archives.postgresql.org/message-id/[EMAIL PROTECTED]

This version fixes a couple bugs in my last patch related to reltime output and
with the new GUC variable, and updated the regression tests to adjust the
new IntervalStyle guc to match the output of the previous regression tests
where the interval output depended on DateStyle.

I've also added it to the Nov CommitFest wiki page.



*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***
*** 4090,4095  SET XML OPTION { DOCUMENT | CONTENT };
--- 4090,4117 
/listitem
   /varlistentry
  
+  varlistentry id=guc-intervalstyle xreflabel=IntervalStyle
+   termvarnameIntervalStyle/varname (typestring/type)/term
+   indexterm
+primaryvarnameIntervalStyle/ configuration parameter/primary
+   /indexterm
+   listitem
+para
+ Sets the display format for interval values. 
+ The value literalsql_standard/ will output SQL Standard
+ strings when given intervals that conform to the SQL
+ standard (either year-month only or date-time only; and no
+ mixing of positive and negative components).
+ The value literalpostgres/ will output intervals in
+ a format that matches what old releases had output when
+ the DateStyle was set to literal'ISO'/.
+ The value literalpostgres_verbose/ will output intervals in
+ a format that matches what old releases had output when
+ the DateStyle was set to literal'SQL'/.
+/para
+   /listitem
+  /varlistentry
+ 
   varlistentry id=guc-timezone xreflabel=timezone
termvarnametimezone/varname (typestring/type)/term
indexterm
*** a/doc/src/sgml/datatype.sgml
--- b/doc/src/sgml/datatype.sgml
***
*** 2213,2218  January 8 04:05:06 1999 PST
--- 2213,2305 
  /para
 /sect2
  
+sect2 id=interval-output
+ titleInterval Output/title
+ 
+ indexterm
+  primaryinterval/primary
+  secondaryoutput format/secondary
+  seealsoformatting/seealso
+ /indexterm
+ 
+ para
+  The output format of the interval types can be set to one of the four
+  styles literalsql_standard/, 
+  literalpostgres/, or literalpostgres_verbose/.The default
+  is the literalpostgres/ format.  
+  xref
+  linkend=interval-style-output-table shows examples of each
+  output style.
+ /para
+ 
+ para
+  The literalsql_standard/ style will output SQL standard
+  interval literal strings where the value of the interval
+  value consists of only a year-month component or a datetime
+  component (as required by the sql standard).   For an interval
+  containing both a year-month and a datetime component, the
+  output will be a SQL Standard unquoted year-month literal
+  string joined to a SQL Standard unquoted datetime literal
+  string with a space in between.
+ /para
+ 
+ para
+  The literalpostgres/ style will output intervals that match
+  the style PostgreSQL 8.3 outputed when the xref linkend=guc-datestyle
+  parameter was set to literalISO/.
+ /para
+ 
+ para
+  The literalpostgres_verbose/ style will output intervals that match
+  the style PostgreSQL 8.3 outputed when the xref linkend=guc-datestyle
+  parameter was set to literalSQL/.
+ /para
+ 
+  table id=interval-style-output-table
+ 	   titleInterval Style Example/title
+ 	   tgroup cols=2
+ 		thead
+ 		 row
+ 		  entryStyle Specification/entry
+ 		  entryYear-Month Interval/entry
+ 		  entryDateTime Interval/entry
+ 		  entryNonstandardrd Extended Interval/entry
+ 		 /row
+ 		/thead
+ 		tbody
+ 		 row
+ 		  entrysql_standard/entry
+ 		  entry1-2/entry
+ 		  entry3 4:05:06/entry
+ 		  entry-1-2 +3 -4:05:06/entry
+ 		 /row
+ 		 row
+ 		  entrypostgres/entry
+ 		  entry1 year 2 mons/entry
+ 		  entry3 days 04:05:06/entry
+ 		  entry -1 years -2 mons +3 days -04:05:06/entry
+ 		 /row
+ 		 row
+ 		  entrypostgres_verbose/entry
+ 		  entry@ 1 year 2 mons/entry
+ 		  entry@ 3 days 4 hours 5 mins 6 secs/entry
+ 		  entry@ 1 year 2 mons -3 days 4 hours 5 mins 6 secs ago/entry
+ 		 /row
+ 		/tbody
+ 	 /tgroup
+ 	/table
+ 
+  para
+  Note that literalsql_standard/ style will only produce strictly 
+  standards-conforming string sliterals when given a strictly SQL-standard interval
+  value - meaning that it needs to be a pure year-month or datetime
+  interval and not mix positive and negative components.
+  /para
+ 
+/sect2
+ 
+ 
+ 
 sect2 id=datatype-timezones
  titleTime Zones/title
  
*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c

Re: [HACKERS] Do we really need a 7.4.22 release now?

2008-09-18 Thread Ron Mayer


Steve Crawford wrote:

Tom Lane wrote:


Yeah.  What this is about is how long the *community* supports 7.4...


Is there any way to poll the community and see how much people
in the community care about 7.4 community support?

It seems possible that most people with large important 7.4 systems
either (a) have commercial support contracts anyway or (b) are capable
of supporting it in-house, or (c) are secretly praying for an excuse
to upgrade anyway.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-standard negative valued year-month literals

2008-09-17 Thread Ron Mayer


Tom Lane wrote:

Stephen R. van den Berg [EMAIL PROTECTED] writes:

Intervals are a scalar, not an addition of assorted values, alternating signs
between fields would be wrong.


Sorry, you're the one who's wrong on that.  We've treated intervals as
three independent fields for years now (and before that it was two
independent fields).  We're not going to throw away that capability.


+1 It's very useful.

Currently our terse input format that's similar to the SQL standard
rejects more mixed-sign intervals than I'd like.  I'd be quite
happy if:
  '1 2:03:-04'
gave me
  '1 day 2 hours 3 minutes -4 seconds'
but currently we reject that mixed-sign-literal.


I'd just like to find a way to have SQL-standard input produce SQL-standard
output in the cases where the input happened to match the standard.

If we had a blank slate, my vote would be that
  '-1 2:03:04'  should mean what the SQL standard says it should.
  '-1 +2:03:04' should mean negative 1 days, plus 2 hours 3 minutes 4 sec
  '1 2:03:-04'  should mean 1 day 2 hours 3 minutes minus 4 seconds
  '-1 2:03:+04'  should mean negative 1 day 2 hours 3 minutes plus 4 seconds
but I'm aware that there are backward compatibility issues.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle

2008-09-17 Thread Ron Mayer

 linkend=guc-datestyle
+  parameter was set to literalISO/.
+ /para
+ 
+ para
+  The literalpostgres_verbose/ style will output intervals that match
+  the style PostgreSQL 8.3 outputed when the xref linkend=guc-datestyle
+  parameter was set to literalSQL/.
+ /para
+ 
+  table id=interval-style-output-table
+ 	   titleInterval Style Example/title
+ 	   tgroup cols=2
+ 		thead
+ 		 row
+ 		  entryStyle Specification/entry
+ 		  entryYear-Month Interval/entry
+ 		  entryDateTime Interval/entry
+ 		  entryNonstandardrd Extended Interval/entry
+ 		 /row
+ 		/thead
+ 		tbody
+ 		 row
+ 		  entrysql_standard/entry
+ 		  entry1-2/entry
+ 		  entry3 4:05:06/entry
+ 		  entry-1-2 +3 -4:05:06/entry
+ 		 /row
+ 		 row
+ 		  entrypostgres/entry
+ 		  entry1 year 2 mons/entry
+ 		  entry3 days 04:05:06/entry
+ 		  entry -1 years -2 mons +3 days -04:05:06/entry
+ 		 /row
+ 		 row
+ 		  entrypostgres_verbose/entry
+ 		  entry@ 1 year 2 mons/entry
+ 		  entry@ 3 days 4 hours 5 mins 6 secs/entry
+ 		  entry@ 1 year 2 mons -3 days 4 hours 5 mins 6 secs ago/entry
+ 		 /row
+ 		/tbody
+ 	 /tgroup
+ 	/table
+ 
+  para
+  Note that literalsql_standard/ style will only produce strictly 
+  standards-conforming string sliterals when given a strictly SQL-standard interval
+  value - meaning that it needs to be a pure year-month or datetime
+  interval and not mix positive and negative components.
+  /para
+ 
+/sect2
+ 
+ 
+ 
 sect2 id=datatype-timezones
  titleTime Zones/title
  
*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c
***
*** 229,234  assign_datestyle(const char *value, bool doit, GucSource source)
--- 229,271 
  
  
  /*
+  * assign_intervalstyle: GUC assign_hook for datestyle
+  */
+ const char *
+ assign_intervalstyle(const char *value, bool doit, GucSource source)
+ {
+ 	int	newIntervalStyle = IntervalStyle;
+ 	char *	result = (char *) malloc(32);
+ 	if (pg_strcasecmp(value, postgres) == 0)
+ 	{
+ 		newIntervalStyle = INTSTYLE_POSTGRES;
+ 	}
+ 	else if (pg_strcasecmp(value, postgres_verbose) == 0)
+ 	{
+ 		newIntervalStyle = INTSTYLE_POSTGRES_VERBOSE;
+ 	}
+ 	else if (pg_strcasecmp(value, sql_standard) == 0)
+ 	{
+ 		newIntervalStyle = INTSTYLE_SQL_STANDARD;
+ 	}
+ 	else
+ 	{
+ 		ereport(GUC_complaint_elevel(source),
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+  errmsg(unrecognized \intervalstyle\ key word: \%s\,
+ 			value)));
+ 		return NULL;
+ 	}
+ 	if (doit)
+ 	{
+ 		IntervalStyle = newIntervalStyle;
+ 		strcpy(result, ISO);
+ 	}
+ 	return result;
+ }
+ 
+ 
+ /*
   * TIMEZONE
   */
  
*** a/src/backend/utils/adt/datetime.c
--- b/src/backend/utils/adt/datetime.c
***
*** 3605,3610  EncodeDateTime(struct pg_tm * tm, fsec_t fsec, int *tzp, char **tzn, int style,
--- 3605,3624 
  	return TRUE;
  }
  
+ /*
+  * small helper funciton to avoid copypaste of this ifdef below
+  */
+ void
+ AppendFsec(char * cp,fsec_t fsec) {
+ if (fsec==0) return;
+ #ifdef HAVE_INT64_TIMESTAMP
+ sprintf(cp, .%06d, Abs(fsec));
+ #else
+ sprintf(cp, :%012.9f, fabs(fsec));
+ #endif
+ TrimTrailingZeros(cp);
+ }
+ 
  
  /* EncodeInterval()
   * Interpret time structure as a delta time and convert to string.
***
*** 3613,3618  EncodeDateTime(struct pg_tm * tm, fsec_t fsec, int *tzp, char **tzn, int style,
--- 3627,3643 
   * Actually, afaik ISO does not address time interval formatting,
   *	but this looks similar to the spec for absolute date/time.
   * - thomas 1998-04-30
+  * 
+  * Actually, afaik, ISO 8601 does specify formats for time
+  * intervals...[of the]...format with time-unit designators, which
+  * are pretty ugly.  The format looks something like
+  * P1Y1M1DT1H1M1.12345S
+  * but useful for exchanging data with computers instead of humans.
+  * - ron 2003-07-14
+  * 
+  * And ISO's SQL 2008 standard specifies standards for
+  * year-month literals (that look like '2-3') and 
+  * day-time literals (that look like ('4 5:6:7')
   */
  int
  EncodeInterval(struct pg_tm * tm, fsec_t fsec, int style, char *str)
***
*** 3621,3626  EncodeInterval(struct pg_tm * tm, fsec_t fsec, int style, char *str)
--- 3646,3658 
  	bool		is_nonzero = FALSE;
  	char	   *cp = str;
  
+ int year  = tm-tm_year;
+ int mon   = tm-tm_mon;
+ int mday  = tm-tm_mday;
+ int hour  = tm-tm_hour;
+ int min   = tm-tm_min;
+ int sec   = tm-tm_sec;
+ 
  	/*
  	 * The sign of year and month are guaranteed to match, since they are
  	 * stored internally as month. But we'll need to check for is_before and
***
*** 3628,3635  EncodeInterval(struct pg_tm * tm, fsec_t fsec, int style, char *str)
  	 */
  	switch (style)
  	{
! 			/* compatible with ISO date formats */
! 		case USE_ISO_DATES:
  			if (tm-tm_year != 0)
  			{
  sprintf(cp, %d year%s,
--- 3660,3738 
  	 */
  	switch (style)
  	{
! 	/* SQL Standard

[HACKERS] Patch for SQL-standard negative valued year-month literals

2008-09-16 Thread Ron Mayer


Tom Lane wrote:

...  SQL-spec interval literals.  I decided to go look at exactly
how unfinished it was, and it turns out that it's actually pretty close.
Hence the attached proposed patch ;-)


Short summary:

 I think this patch fixes a bug with sql-spec negative interval literals.

Longer.

I believe this short (4 lines of code  1 line of comment) patch (below)
fixes the way we handle SQL-standard negative-valued year-month interval
strings.

In particular, if I read the spec right (relevant excerpts below), the
string '-1-1' is a vaild SQL-200N year-month interval meaning a
negative year and a month - because the spec only allows a sign
in the beginning of the unquoted interval string:
   unquoted interval string ::=
 [ sign ] { year-month literal | day-time literal }
Current HEAD interprets '-1-1' as -1 days -1 hours.  8.3 doesn't
recognize it at all.

Assuming I read the spec right, are there any problems with this,
and if not, could I ask that the patch at the end of this email
be applied?

   Ron



===
== with this patch
===

regression=# select interval '-1-1';
 interval
--
 -1 years -1 mons
(1 row)

===
== without this patch
===

regression=# select interval '-1-1';
 interval
---
 -1 days -01:00:00
(1 row)

===
== 8.3
===

regression=# select interval '-1-1';
ERROR:  invalid input syntax for type interval: -1-1

===
== I think the relevant part of SQL 200N
===

interval string ::=
  quote unquoted interval string quote

unquoted interval string ::=
  [ sign ] { year-month literal | day-time literal }

year-month literal ::=
years value [ minus sign months value ]
  | months value

years value ::=
  datetime value
months value ::=
  datetime value

datetime value ::=
  unsigned integer

... If SV is a negative interval, then sign shall be specified
within unquoted interval string in the literal Y.

===
== The patch
===

*** a/src/backend/utils/adt/datetime.c
--- b/src/backend/utils/adt/datetime.c
***
*** 609,621  ParseDateTime(const char *timestr, char *workbuf, size_t 
buflen,
/* soak up leading whitespace */
while (isspace((unsigned char) *cp))
cp++;
!   /* numeric timezone? */
if (isdigit((unsigned char) *cp))
{
ftype[nf] = DTK_TZ;
APPEND_CHAR(bufp, bufend, *cp++);
while (isdigit((unsigned char) *cp) ||
!  *cp == ':' || *cp == '.')
APPEND_CHAR(bufp, bufend, *cp++);
}
/* special? */
--- 609,621 
/* soak up leading whitespace */
while (isspace((unsigned char) *cp))
cp++;
!   /* numeric timezone?  or sql year-month interval?*/
if (isdigit((unsigned char) *cp))
{
ftype[nf] = DTK_TZ;
APPEND_CHAR(bufp, bufend, *cp++);
while (isdigit((unsigned char) *cp) ||
!  *cp == ':' || *cp == '.' || *cp == 
'-')
APPEND_CHAR(bufp, bufend, *cp++);
}
/* special? */
***
*** 2876,2889  DecodeInterval(char **field, int *ftype, int nf, int range,
{
/* SQL years-months syntax */
int val2;
!
val2 = strtoi(cp + 1, cp, 10);
if (errno == ERANGE || val2  0 || val2 
= MONTHS_PER_YEAR)
return DTERR_FIELD_OVERFLOW;
if (*cp != '\0')
return DTERR_BAD_FORMAT;
type = DTK_MONTH

Re: [HACKERS] Patch for SQL-standard negative valued year-month literals

2008-09-16 Thread Ron Mayer


Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

Short summary:
  I think this patch fixes a bug with sql-spec negative interval literals.


Hmm.  I'm a bit concerned about possible side-effects on other cases:
what had been seen as two separate tokens will now become one token
for *all* datetime types, not just interval.  However, I can't


If it's a concern, I could make interval_in first look for the
SQL-standard patterns before even parsing the string into fields.
If we want to handle the SQL standard negative datetime intervals
(see below) the way the spec looks to me,


immediately think of any likely input formats where this would be a
problem.
Something else I noticed while poking at it is this inconsistency:...


Yes. I saw some of those too (and '-1 1:00:00'); but didn't have a
patch ready (yet).   I'm happy to work on it.


As soon as you throw in a sign, it gets wacky :-(.


Oh.  And looking more closely; there's a potential bad incompatibility.

If I read SQL 200N's spec correctly

 select interval '-1 1:00:00';

should mean-1 days -1 hours,
yet 8.3 sees it as -1 days +1 hours.

Scary to touch that one, but since a standard's a standard, I think
eventually we should get there.   Perhaps we need a GUC to choose
standards or backward compatible behavior for that one?   Or perhaps
keep parsing it the old way but with a WARNING for 8.4 and switch in 8.5?


The reason for this bizarreness is this chunk of code at the end of
DecodeInterval's DTK_TZ case:

else if (type == IGNORE_DTF)
{...} }

which means that a signed integer gets forced to be hours if there
isn't an explicit unit spec in the literal, while a signed float gets
forced to be seconds.  I can't see any reason why that's a good idea,
and propose that while we're making incompatible changes in corner cases,
we should just drop the code quoted above.


+1.   Shall I try for a patch that handles those and possibly the (more
frightening) SQL 200N signed day-time interval mentioned above.





--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Patch for SQL-standard negative valued year-month literals

2008-09-16 Thread Ron Mayer


Tom Lane wrote:

If I read SQL 200N's spec correctly
  select interval '-1 1:00:00';
should mean-1 days -1 hours,
yet 8.3 sees it as -1 days +1 hours.


I think we are kind of stuck on this one.  If we change it, then how
would one represent -1 days +1 hours?  The spec's format is only sane


I'm not proposing this, but I could imagine making
-1 -1:00:00 and/or -1 +1:00:00 mean -1 days +1 hours.

I think if it weren't for backward compatibility issues I'd even
support such an idea - since now we're oh-so-very-close to accepting
to-spec literals.  Unfortunately I fear old users might assume the
opposite meaning.


Scary to touch that one, but since a standard's a standard, I think
eventually we should get there.


The SQL spec's date/time handling is, and always has been, broken enough
that I feel no great compulsion to follow every last detail.  Especially
details that make it impossible to support our extensions...


In this case we're so very close to meeting the spec, though.  And
it's ashame where we accept the to-spec syntax (-1 1:00:00) but
return a different answer.

And since it's not quite impossible to support our extensions (force
putting the sign on the h:m:s part for mixed sign cases) I feels nice
to me to try.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] 8.3 vs HEAD difference in Interval output?



Unless I'm compiling stuff wrong, it seems HEAD is giving me
slightly different output on Intervals than 8.3 in the roundoff
of seconds.   8.3 was rounding to the nearest fraction of a second,
HEAD seems to be truncating.

In the psql output below it shows 8.3.1 outputting 6.70 secs
while the similar output for head is showing 6.69 secs.

At first glance it seems this might be because HEAD defaults
to USE_INTEGER_DATETIMES, which leads to HAVE_INT64_TIMESTAMP
which leads to
  sprintf(cp, %s%d.%02d secs, is_nonzero ?   : ,
  tm-tm_sec, ((int) sec) / 1);
in EncodeInterval in datetime.c which doesn't seem to be
doing any rounding.

Am I interpreting this right?  If so, shall I submit a patch
that rounds it to hundredths of a second (hundredths seems
hardcoded in the sprintf), or perhaps just silently add that
fix to the EncodeInterval patch I'm doing any for SQL Standard
and ISO intervals?




psql (8.4devel)
Type help for help.
regression=# set datestyle to sql;
SET
regression=#  select '1 year 2 mons 3 days 04:05:06.69'::interval;
interval
-
 @ 1 year 2 mons 3 days 4 hours 5 mins 6.69 secs
(1 row)


Welcome to psql 8.3.1, the PostgreSQL interactive terminal.
...
pg83=# set datestyle to sql;
SET
pg83=#  select '1 year 2 mons 3 days 04:05:06.69'::interval;
interval
-
 @ 1 year 2 mons 3 days 4 hours 5 mins 6.70 secs
(1 row)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] 8.3 vs HEAD difference in Interval output?


Kevin Grittner wrote:

...not the only place where the float-timestamps code has
rounding behavior that doesn't appear in the integer-timestamps 
code. ...  
I find the results on 8.3.3 with integer timestamps surprising:


Agreed it's surprising and agreed there are more places.

Sounds like I should keep that separate and perhaps later
submit a separate patch to identify and/or remove surprising
rounding behavior.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] 8.3 vs HEAD difference in Interval output?


Tom Lane wrote:


This is not the only place where the float-timestamps code has rounding
behavior that doesn't appear in the integer-timestamps code. 


Yeah... For that matter, I find this surprising as well:

regression=# select '0.7 secs'::interval, ('7 secs'::interval/10);
interval |  ?column?
-+-
 00:00:00.69 | 00:00:00.70
(1 row)

I'll start making a list of them for a future patch down the road.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposed patch: make SQL interval-literal syntax work per spec


Tom Lane wrote:

support for SQL-spec interval literals.  I decided to go look at exactly
how unfinished it was, and it turns out that it's actually pretty close.
Hence the attached proposed patch ;-)


Is this code handling negative interval literals right?
I think I quote the relevant spec part at the bottom.


If I'm reading the spec right, I find this surprising.

regression=# select interval '-1-1';
 interval
---
 -1 days -01:00:00

regression=# select interval '1-1';
   interval
--
 1 year 1 mon


Also if I read the spec right, ISTM the sign should
apply to the entire interval literal.  But if I compiled
the patch right, the negative sign applies only to the
first part it encounters?

regression=# select interval '-1 2:3:4';
 interval
---
 -1 days +02:03:04
(1 row)


If I understand right, this'll add confusion to
SQL standard output for mixed year-month and
day-time intervals that have both negative and
positive components too. :(


This looks to me like the relevant part of SQL 200N(8?),
which seems to me to allow a sign inside the quotes:

interval literal ::=
   INTERVAL [ sign ] interval string interval qualifier
interval string ::=
   quote unquoted interval string quote
unquoted interval string ::=
   [ sign ] { year-month literal | day-time literal }




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposed patch: make SQL interval-literal syntax work per spec


Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

Is this code handling negative interval literals right?
I think I quote the relevant spec part at the bottom.


We support independent signs for the different components of the


Even so it surprises me that:
'-1-1'::interval gives me a day-hour interval while
'1-1'::interval gives me a year-month interval.



I'm not sure how well the spec copes with that.


If I'm read the spec right, SQL 2008  expects -1 12:34:56 to
be what we'd see as -1 -12:34:56, and doesn't handle with
different signs for different components at all.

SQL 92 seems to have been simpler, apparently requiring
the negative sign to stay outside the quotes.
==SQL 92===
interval literal ::=
INTERVAL [ sign ] interval string interval qualifier
interval string ::=
   quote { year-month literal | day-time literal } quote
===

SQL 200N seems to allow a sign inside the string:
==SQL 200N=
interval literal ::=
   INTERVAL [ sign ] interval string interval qualifier
interval string ::=
   quote unquoted interval string quote
unquoted interval string ::=
   [ sign ] { year-month literal | day-time literal }
===


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposed patch: make SQL interval-literal syntax work per spec

2008-09-13 Thread Ron Mayer


Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

interval ... sql_standard...iso_8601...
backward_compatible ...depends... on ... DateStyle...


...How about decoupling interval_out's behavior
from DateStyle altogether, and instead providing values of IntervalStyle
that match all the previous behaviors?


Great.  That seems much more sane.

Any desired names for the existing interval styles?

Currently we output intervals in these two styles:
 '1 year 2 mons 3 days 04:05:06'
 when the DateStyle is iso.
and
 '@ 1 year 2 mons 3 days 4 hours 5 mins 6 secs'
 when the DateStyle is sql or postgres, etc.

I'm not quite sure where those styles came from so
don't know what good names for them might be.


Should those ECPG functions be made identical ...

...
The palloc and elog dependencies seem to be the hard part.


Interesting.   So EncodeDateTime and EncodeInterval, guessing 400 or
so lines, seem sharable since at first glance they either already do or
easily could make their callers deal with all allocation and logging.

Agreed that it's a independent patch that I'll try separately.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposed patch: make SQL interval-literal syntax work per spec


Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

... ISO 8601 intervals ...


On the output side, seems like a GUC variable
is the standard precedent here.  I'd still vote against overloading
DateStyle --- it does too much already --- but a separate variable for
interval style wouldn't bother me.  In fact, given that we are now
somewhat SQL-compliant on interval input, a GUC that selected
PG traditional, SQL-standard, or ISO 8601 interval output format seems
like it could be a good idea.


Is it OK that this seems to me it wouldn't be backward compatible
with the current interval_out that looks to me to be using
the DateStyle GUC?

I supposed it could be made backward compatible if the new
IntervalStyle GUC defaulted to a value of guess_from_datestyle,
but I fear an option like that  might add rather than remove
confusion.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposed patch: make SQL interval-literal syntax work per spec


Tom Lane wrote:

somewhat SQL-compliant on interval input, a GUC that selected
PG traditional, SQL-standard, or ISO 8601 interval output format seems
like it could be a good idea.


Trying to do the SQL-standard output now, and have a question
of what to do in the SQL-standard mode when trying to output
an interval that as both a YEAR and a DAY component.

AFAICT the SQL standard doesn't let you have both, so the
SQL-standard output actually won't be.



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposed patch: make SQL interval-literal syntax work per spec


Tom Lane wrote:

The reason it's not SQL-standard is the data value isn't.
So not a problem.  Someone conforming to the spec limits on
what he puts in will see spec-compliant output.  I think all
you need is 'yyy-mm dd hh:mm:ss' where you omit yyy-mm if
zeroes, omit dd if zero, omit hh:mm:ss if zeroes (but maybe
only if dd is also 0?  otherwise your output is just dd which
is uncomfortably ambiguous).


Great.  That's what I'll do.

Any convention or preference on the naming of the GUC?
I assume intervalstyle is reasonable?

Or thoughts regarding the current EncodeInterval() that's
already using the datestyle GUC?


pg82=# select interval '1';
 interval
--
 00:00:01
(1 row)

pg82=# set datestyle='sql';
SET

pg82=# select interval '1';
 interval
--
 @ 1 sec
(1 row)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposed patch: make SQL interval-literal syntax work per spec


Ron Mayer wrote:

Tom Lane wrote:

you need is 'yyy-mm dd hh:mm:ss' where you omit yyy-mm if
zeroes, omit dd if zero, omit hh:mm:ss if zeroes (but maybe
only if dd is also 0?  otherwise your output is just dd which
is uncomfortably ambiguous).


Oh, and if both parts are 0, I guess we desire
the (more comfortable than the alternatives) '0'?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposed patch: make SQL interval-literal syntax work per spec


Tom Lane wrote:

I think all
you need is 'yyy-mm dd hh:mm:ss' where you omit yyy-mm if
zeroes, omit dd if zero, omit hh:mm:ss if zeroes (but maybe
only if dd is also 0?  otherwise your output is just dd which
is uncomfortably ambiguous).


Cool.  I think I have it pretty much working with a new
GUC intervalstyle that can take values of

sql_standard that I think will output SQL standard
   interval literals when given a sql
   standard interval.

iso_8601 that will output ISO 8601 Time Intervals of
   the format with time-unit deignators, and

backward_compatible that will output the same thing
   that postgres currently does that depends
   on the value of the DateStyle GUC.

I'll add the documentation and regression tests and
can submit a patch early next week.   Oh.  One more
question is that under ecpg there seems to be a fair
amount of near-duplicated code (EncodeDateTime,
EncodeInterval) for turning dates and times and
intervals to strings.

Should those ECPG functions be made identical to
the ones in the backend?
Could those somehow share code with the backend for
some of their work?


Anyway - here's a quick test of the
SQL Standard and ISO interval output as it stands
right now...



regression=# drop table test_intervals;
DROP TABLE
regression=# create temporary table test_intervals (i interval);
CREATE TABLE
regression=# insert into test_intervals values
regression-#   ('0 years'),
regression-#   ('1 year 1 month'),
regression-#   ('1 day 2 hours 3 minutes 4 seconds'),
regression-#   ('1 year 1 minute');
INSERT 0 4
regression=#
regression=# insert into test_intervals values
regression-#   ('1-1'),
regression-#   ('1'),
regression-#   (interval '1' year),
regression-#   ('1:00:00'),
regression-#   ('1 1:02:03');
INSERT 0 5
regression=#
regression=# insert into test_intervals values
regression-#   ('P1Y1M'),
regression-#   ('P1DT1H1M1S'),
regression-#   ('PT1S');
INSERT 0 3
regression=#
regression=# set intervalstyle to sql_standard;
SET
regression=# select * from test_intervals;
  i
-
 0
 1-1
 1 2:3:4
 1-0 0 0:1:0
 1-1
 0:0:1
 1-0
 1:0:0
 1 1:2:3
 1-1
 1 1:1:1
 0:0:1
(12 rows)


regression=#
regression=# set intervalstyle to iso_8601;
SET
regression=# select * from test_intervals;
 i

 PT0S
 P1Y1M
 P1DT2H3M4S
 P1YT1M
 P1Y1M
 PT1S
 P1Y
 PT1H
 P1DT1H2M3S
 P1Y1M
 P1DT1H1M1S
 PT1S
(12 rows)

regression=#
regression=# set intervalstyle to backward_compatible;
SET
regression=# set datestyle to sql;
SET
regression=# select * from test_intervals;
   i
---
 @ 0
 @ 1 year 1 mon
 @ 1 day 2 hours 3 mins 4 secs
 @ 1 year 1 min
 @ 1 year 1 mon
 @ 1 sec
 @ 1 year
 @ 1 hour
 @ 1 day 1 hour 2 mins 3 secs
 @ 1 year 1 mon
 @ 1 day 1 hour 1 min 1 sec
 @ 1 sec
(12 rows)

regression=# set datestyle to iso;
SET
regression=# select * from test_intervals;
i
-
 00:00:00
 1 year 1 mon
 1 day 02:03:04
 1 year 00:01:00
 1 year 1 mon
 00:00:01
 1 year
 01:00:00
 1 day 01:02:03
 1 year 1 mon
 1 day 01:01:01
 00:00:01
(12 rows)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposed patch: make SQL interval-literal syntax work per spec


Tom Lane wrote:

Kevin Grittner [EMAIL PROTECTED] writes:
Tom Lane [EMAIL PROTECTED] wrote: 

I am not sure about some of the corner cases --- anyone want to see if
their understanding of the spec for interval string is different?
 

The patch seems to support extensions to the standard.


Right.  All of these were extensions that already existed in PG.


Back a while ago (2003) there was some talk about replacing
some of the non-standard extensions with shorthand forms of
intervals with ISO 8601 intervals that have a similar but
not-the-same shorthand.
 IntervalISO Postgres
 8601shorthand
 -
 '1 year 1 minute'   'P1YT1M' '1Y1M'
 '1 year 1 month''P1Y1M'  N/A
http://archives.postgresql.org/pgsql-patches/2003-09/msg00119.php

When I proposed we support the ISO-8601 standard shorthand,
Tom recommended we rip out the old shorthand at the same time:

http://archives.postgresql.org/pgsql-patches/2003-09/msg00134.php

I've been maintaining a patch that supports these ISO-8601
intervals for a client.  Back in 2003 I recall Peter said
he wanted to see SQL standard intervals first.  There were
also discussions about selecting the output format.  My old
patch made this depend on datestyle; but it seems Tom preferred
a separate GUC?

http://archives.postgresql.org/pgsql-patches/2003-12/msg00257.php

I see there's a TODO regarding ISO8601 intervals as well.

I have a version of this patch for 8.2; but would be happy
to bring it up-to-date if people want to re-consider it now.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposed patch: make SQL interval-literal syntax work per spec


Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

Back a while ago (2003) there was some talk about replacing
some of the non-standard extensions with shorthand forms of
intervals with ISO 8601 intervals that have a similar but
not-the-same shorthand.


I think *replacement* would be a hard sell, as that would tick off all
the existing users ;-).  Now it seems like being able to accept either


I originally submitted a patch that supported both, and I think
you suggested replacing on the grounds that the old one was
never documented,

http://archives.postgresql.org/pgsql-patches/2003-09/msg00134.php
 If we're going to support the real ISO spec, I'd suggest ripping
  out any not-quite-there variant.

http://archives.postgresql.org/pgsql-patches/2003-09/msg00121.php
 I doubt anyone is using it, because it's completely undocumented.

On the other hand, the company I was at was indeed originally
using it, so I prefer that it stay in as well.   Perhaps if
there's a way to mark them as deprecated and post warnings in
the log file if they're used.   I think they should be
removed eventually in a few releases, because they're quite
confusing as they stand:

 IntervalISO Postgres
 8601shorthand
 -
 '1 year 1 minute'   'P1YT1M' '1Y1M'
 '1 year 1 month''P1Y1M'  N/A



the 8601 syntax or the existing syntaxes on input wouldn't be tough
at all, if you insist on the P prefix to distinguish; so that end of


ISO 8601 seems to me to require the P, so I think we would.


it should be easy enough.  On the output side, seems like a GUC variable
is the standard precedent here.  I'd still vote against overloading
DateStyle --- it does too much already --- but a separate variable for
interval style wouldn't bother me.  In fact, given that we are now
somewhat SQL-compliant on interval input, a GUC that selected
PG traditional, SQL-standard, or ISO 8601 interval output format seems
like it could be a good idea.


Great.  I'm bringing my patch up-to-date with CVS now
and adding the separate GUC.



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposed patch: make SQL interval-literal syntax work per spec


Tom Lane wrote:


The other problem is that the SQL spec clearly defines an interval
literal syntax, and it's not this ISO thing.  So even without backward
compatibility issues, 8601-only doesn't seem like it would fly.



Oh.  I wasn't proposing 8601-only.  Just the one-character
shorthands like '1Y1M'::interval that postgresql interprets
as 1 year one minute.   No standard specifies anything close
to that; and any similar standards ask to interpret that M as
months instead of minutes.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposed patch: make SQL interval-literal syntax work per spec