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 cut&paste 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 cut&paste 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
