Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle - Mailing list pgsql-hackers

From Ron Mayer
Subject Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle
Date
Msg-id 48D15471.6080305@cheapcomplexdevices.com
Whole thread Raw
In response to Re: Proposed patch: make SQL interval-literal syntax work per spec  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle  (Ron Mayer <rm_pg@cheapcomplexdevices.com>)
Patch for ISO-8601-Interval Input and output.  (Ron Mayer <rm_pg@cheapcomplexdevices.com>)
Re: Patch for SQL-Standard Interval output and decoupling DateStyle from IntervalStyle  ("Brendan Jurd" <direvus@gmail.com>)
List pgsql-hackers
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.

Short summary:

    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).

Background:

    Currently Postgres outputs Intervals in one of two formats
    depending on DateStyle.  When DateStyle is 'ISO',  it outputs
    intervals like '1 year 2 mons 3 days -04:05:06.78' (though I
    know of no ISO interval standards that look like that).  When
    DateStyle is 'SQL' it outputs intervals like
    '@ 1 year 2 mons 3 days -4 hours -5 mins -6.78 secs' (though
    I know of no SQL interval standards that look like that).

The feature:

    The SQL standard only specifies interval literal strings
    for the two kinds of intervals (year-month and day-time)
    that are defined by the SQL spec.  It also doesn't account
    for postgres's intervals that can have mixed-sign components.
    I tried to make the output here be a logical extension
    of the spec, concatenating the year-month and day-time
    interval strings and forcing signs in the output that could
    otherwise have been ambiguous.

    This little table shows an example of the output of this
    new style compared to the existing postgres output for
    (a) a year-month interval, (b) a day-time interval, and
    (c) a not-quite-standard postgres interval with year-month
    and day-time components of varying signs.

     '1-2'                 | '@ 1 year 2 mons'
     '3 4:05:06.78'        | '@ 3 days 4 hours 5 mins 6.78 secs'
     '+1-2 -3 +4:05:06.78' | '@ 1 year 2 mons -3 days 4 hours 5 mins 6.78 secs'

The patch:

    Seems to work for me; and I believe I updated the docs.
    Many regression tests fail, though, because they assume
    the existing coupling of DateStyle and interval output
    styles.   If people like where this is going I can update
    those regression tests and add ones to test this new style.


*** 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">
+       <term><varname>IntervalStyle</varname> (<type>string</type>)</term>
+       <indexterm>
+        <primary><varname>IntervalStyle</> configuration parameter</primary>
+       </indexterm>
+       <listitem>
+        <para>
+         Sets the display format for interval values.
+         The value <literal>sql_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 <literal>postgres</> will output intervals in
+         a format that matches what old releases had output when
+         the DateStyle was set to <literal>'ISO'</>.
+         The value <literal>postgres_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">
        <term><varname>timezone</varname> (<type>string</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">
+     <title>Interval Output</title>
+
+     <indexterm>
+      <primary>interval</primary>
+      <secondary>output format</secondary>
+      <seealso>formatting</seealso>
+     </indexterm>
+
+     <para>
+      The output format of the interval types can be set to one of the four
+      styles <literal>sql_standard</>,
+      <literal>postgres</>, or <literal>postgres_verbose</>.The default
+      is the <literal>postgres</> format.
+      <xref
+      linkend="interval-style-output-table"> shows examples of each
+      output style.
+     </para>
+
+     <para>
+      The <literal>sql_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 <literal>postgres</> style will output intervals that match
+      the style PostgreSQL 8.3 outputed when the <xref linkend="guc-datestyle">
+      parameter was set to <literal>ISO</>.
+     </para>
+
+     <para>
+      The <literal>postgres_verbose</> style will output intervals that match
+      the style PostgreSQL 8.3 outputed when the <xref linkend="guc-datestyle">
+      parameter was set to <literal>SQL</>.
+     </para>
+
+      <table id="interval-style-output-table">
+        <title>Interval Style Example</title>
+        <tgroup cols="2">
+         <thead>
+          <row>
+           <entry>Style Specification</entry>
+           <entry>Year-Month Interval</entry>
+           <entry>DateTime Interval</entry>
+           <entry>Nonstandardrd Extended Interval</entry>
+          </row>
+         </thead>
+         <tbody>
+          <row>
+           <entry>sql_standard</entry>
+           <entry>1-2</entry>
+           <entry>3 4:05:06</entry>
+           <entry>-1-2 +3 -4:05:06</entry>
+          </row>
+          <row>
+           <entry>postgres</entry>
+           <entry>1 year 2 mons</entry>
+           <entry>3 days 04:05:06</entry>
+           <entry> -1 years -2 mons +3 days -04:05:06</entry>
+          </row>
+          <row>
+           <entry>postgres_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 <literal>sql_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">
      <title>Time 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 copy&paste 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 literal"s (that look like '2-3') and
+  * "day-time literal"s (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 interval literals */
!         case INTSTYLE_SQL_STANDARD:
!         {
!             bool has_negative = (year < 0) || (mon  < 0) ||
!                                 (mday < 0) || (hour < 0) ||
!                                 (min  < 0) || (sec  < 0) || (fsec<0);
!             bool has_positive = (year > 0) || (mon  > 0) ||
!                                 (mday > 0) || (hour > 0) ||
!                                 (min  > 0) || (sec > 0) || (fsec>0);
!             bool has_year_month = (year != 0) || (mon != 0);
!             bool has_datetime   = (hour != 0) || (min != 0) ||
!                                   (sec  != 0) || (fsec!= 0) || (mday != 0);
!             bool has_day        = (mday != 0);
!             bool sql_standard_value = (!(has_negative && has_positive)) &&
!                                       (!(has_year_month && has_datetime));
!             /*
!              * SQL Standard wants only 1 "<sign>" preceeding the whole
!              * interval.
!              */
!             if (has_negative && sql_standard_value)
!             {
!                 sprintf(cp,"-");
!                 cp++;
!                 year = -year;
!                 mon  = -mon;
!                 mday = -mday;
!                 hour = -hour;
!                 min  = -min;
!                 sec  = -sec;
!                 fsec = -fsec;
!             }
!             if (!has_negative && !has_positive)
!             {
!                 sprintf(cp,"0");
!             }
!             else if (!sql_standard_value)
!             {
!                 /*
!                  * For non sql-standard interval values,
!                  * force outputting the signs to avoid
!                  * ambiguities with intervals with mixed
!                  * sign components.
!                  */
!                 char year_sign = (year<0 || mon<0) ? '-' : '+';
!                 char day_sign = (mday<0) ? '-' : '+';
!                 char sec_sign = (hour<0 || min<0 || sec<0 || fsec<0)
!                                 ? '-' : '+';
!                 sprintf(cp,"%c%d-%d %c%d %c%d:%02d:%02d",
!                         year_sign,abs(year),abs(mon),
!                         day_sign,abs(mday),
!                         sec_sign,abs(hour),abs(min),abs(sec));
!                 AppendFsec(cp+strlen(cp),fsec);
!             }
!             else if (has_year_month)
!             {
!                 sprintf(cp,"%d-%d",year,mon);
!             }
!             else if (has_day)
!             {
!                 sprintf(cp,"%d %d:%02d:%02d",mday,hour,min,sec);
!                 AppendFsec(cp+strlen(cp),fsec);
!             }
!             else
!             {
!                 sprintf(cp,"%d:%02d:%02d",hour,min,sec);
!                 AppendFsec(cp+strlen(cp),fsec);
!             }
!             cp += strlen(cp);
!             break;
!         }
!
!         /* compatible with postgresql 8.3 when DateStyle = 'iso' */
!         case INTSTYLE_POSTGRES:
              if (tm->tm_year != 0)
              {
                  sprintf(cp, "%d year%s",
***************
*** 3692,3700 **** EncodeInterval(struct pg_tm * tm, fsec_t fsec, int style, char *str)
                      cp += strlen(cp);
                  }
              }
              break;

!         case USE_POSTGRES_DATES:
          default:
              strcpy(cp, "@ ");
              cp += strlen(cp);
--- 3795,3809 ----
                      cp += strlen(cp);
                  }
              }
+             if (!is_nonzero)
+             {
+                 strcat(cp, "0");
+                 cp += strlen(cp);
+             }
              break;

!         /* compatible with postgresql 8.3 when DateStyle = 'sql' */
!         case INTSTYLE_POSTGRES_VERBOSE:
          default:
              strcpy(cp, "@ ");
              cp += strlen(cp);
***************
*** 3821,3842 **** EncodeInterval(struct pg_tm * tm, fsec_t fsec, int style, char *str)
                      is_before = (tm->tm_sec < 0);
                  is_nonzero = TRUE;
              }
              break;
      }

-     /* identically zero? then put in a unitless zero... */
-     if (!is_nonzero)
-     {
-         strcat(cp, "0");
-         cp += strlen(cp);
-     }
-
-     if (is_before && (style != USE_ISO_DATES))
-     {
-         strcat(cp, " ago");
-         cp += strlen(cp);
-     }
-
      return 0;
  }    /* EncodeInterval() */

--- 3930,3948 ----
                      is_before = (tm->tm_sec < 0);
                  is_nonzero = TRUE;
              }
+             if (!is_nonzero)
+             {
+                 strcat(cp, "0");
+                 cp += strlen(cp);
+             }
+             if (is_before)
+             {
+                 strcat(cp, " ago");
+                 cp += strlen(cp);
+             }
              break;
      }

      return 0;
  }    /* EncodeInterval() */

*** a/src/backend/utils/adt/nabstime.c
--- b/src/backend/utils/adt/nabstime.c
***************
*** 671,677 **** reltimeout(PG_FUNCTION_ARGS)
      char        buf[MAXDATELEN + 1];

      reltime2tm(time, tm);
!     EncodeInterval(tm, 0, DateStyle, buf);

      result = pstrdup(buf);
      PG_RETURN_CSTRING(result);
--- 671,677 ----
      char        buf[MAXDATELEN + 1];

      reltime2tm(time, tm);
!     EncodeInterval(tm, INTSTYLE_POSTGRES, DateStyle, buf);

      result = pstrdup(buf);
      PG_RETURN_CSTRING(result);
*** a/src/backend/utils/adt/timestamp.c
--- b/src/backend/utils/adt/timestamp.c
***************
*** 677,683 **** interval_out(PG_FUNCTION_ARGS)
      if (interval2tm(*span, tm, &fsec) != 0)
          elog(ERROR, "could not convert interval to tm");

!     if (EncodeInterval(tm, fsec, DateStyle, buf) != 0)
          elog(ERROR, "could not format interval");

      result = pstrdup(buf);
--- 677,683 ----
      if (interval2tm(*span, tm, &fsec) != 0)
          elog(ERROR, "could not convert interval to tm");

!     if (EncodeInterval(tm, fsec, IntervalStyle, buf) != 0)
          elog(ERROR, "could not format interval");

      result = pstrdup(buf);
*** a/src/backend/utils/init/globals.c
--- b/src/backend/utils/init/globals.c
***************
*** 88,93 **** bool        ExitOnAnyError = false;
--- 88,94 ----

  int            DateStyle = USE_ISO_DATES;
  int            DateOrder = DATEORDER_MDY;
+ int            IntervalStyle = INTSTYLE_POSTGRES;
  bool        HasCTZSet = false;
  int            CTimeZone = 0;

*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 367,372 **** static bool session_auth_is_superuser;
--- 367,373 ----
  static double phony_random_seed;
  static char *client_encoding_string;
  static char *datestyle_string;
+ static char *intervalstyle_string;
  static char *locale_collate;
  static char *locale_ctype;
  static char *server_encoding_string;
***************
*** 2098,2103 **** static struct config_string ConfigureNamesString[] =
--- 2099,2113 ----
          "ISO, MDY", assign_datestyle, NULL
      },

+      {
+         {"IntervalStyle", PGC_USERSET, CLIENT_CONN_LOCALE,
+             gettext_noop("Sets the display format for interval values."),
+             GUC_LIST_INPUT | GUC_REPORT
+         },
+         &intervalstyle_string,
+         "postgres", assign_intervalstyle, NULL
+     },
+
      {
          {"default_tablespace", PGC_USERSET, CLIENT_CONN_STATEMENT,
              gettext_noop("Sets the default tablespace to create tables and indexes in."),
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 426,431 ****
--- 426,432 ----
  # - Locale and Formatting -

  #datestyle = 'iso, mdy'
+ #intervalstyle = 'postgres'
  #timezone = unknown            # actually, defaults to TZ environment
                      # setting
  #timezone_abbreviations = 'Default'     # Select the set of available time zone
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 1953,1958 **** psql_completion(char *text, int start, int end)
--- 1953,1965 ----

              COMPLETE_WITH_LIST(my_list);
          }
+         else if (pg_strcasecmp(prev2_wd, "IntervalStyle") == 0)
+         {
+             static const char *const my_list[] =
+             {"postgres","postgres_verbose", "sql_standard", NULL};
+
+             COMPLETE_WITH_LIST(my_list);
+         }
          else if (pg_strcasecmp(prev2_wd, "GEQO") == 0)
          {
              static const char *const my_list[] =
*** a/src/include/commands/variable.h
--- b/src/include/commands/variable.h
***************
*** 15,20 ****
--- 15,22 ----

  extern const char *assign_datestyle(const char *value,
                   bool doit, GucSource source);
+ extern const char *assign_intervalstyle(const char *value,
+                  bool doit, GucSource source);
  extern const char *assign_timezone(const char *value,
                  bool doit, GucSource source);
  extern const char *show_timezone(void);
*** a/src/include/miscadmin.h
--- b/src/include/miscadmin.h
***************
*** 191,196 **** extern PGDLLIMPORT Oid MyDatabaseTableSpace;
--- 191,208 ----

  extern int    DateStyle;
  extern int    DateOrder;
+
+ /*
+  * IntervalStyles
+  *   INTSTYLE_POSTGRES             Like Postgres8.3 when DateStyle = 'iso'
+  *   INTSTYLE_POSTGRES_VERBOSE     Like Postgres8.3 when DateStyle = 'sql'
+  *   INTSTYLE_SQL_STANDARD         SQL standard interals
+  */
+ #define INTSTYLE_POSTGRES             0
+ #define INTSTYLE_POSTGRES_VERBOSE     1
+ #define INTSTYLE_SQL_STANDARD         2
+
+ extern int    IntervalStyle;

  /*
   * HasCTZSet is true if user has set timezone as a numeric offset from UTC.

pgsql-hackers by date:

Previous
From: Teodor Sigaev
Date:
Subject: Re: text search patch status update?
Next
From: Simon Riggs
Date:
Subject: Re: Autovacuum and Autoanalyze