builtins: add parse and to_char_with_style variants for date types #68351

otan · 2021-08-02T23:29:48Z

See individual commits for details.

cockroach-teamcity · 2021-08-02T23:29:58Z

This change is

rafiss

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan)

pkg/sql/logictest/testdata/logic_test/datetime, line 1864 at r2 (raw file):

SELECT
  parse_timestamp(s),
  parse_timestamp(s, 'iso,mdy'),

should this also work with just parse_timestamp(s, 'mdy') ?

pkg/sql/logictest/testdata/logic_test/datetime, line 1887 at r3 (raw file):

  parse_date(s, 'iso,mdy'),
  parse_date(s, 'iso,dmy'),
  parse_date(s, 'iso,ymd')

similar, should this work with parse_date(s, 'mdy') etc?

pkg/sql/logictest/testdata/logic_test/datetime, line 1908 at r4 (raw file):

INSERT INTO time_datestyle_parse VALUES
  (1, '2007-09-12 11:30:45.123+06'),
  (2, '2007-09-12 11:30:45.123+03')

how does DMY ordering affect these tests?

pkg/sql/logictest/testdata/logic_test/interval, line 456 at r1 (raw file):

    to_char_with_style(i, 'iso_8601') AS iso,
    to_char_with_style(i, 'sql_standard') AS sql_std,
    to_char_with_style(i) AS default_std

nit: why default_std? a name like default_style makes more sense to me

pkg/sql/logictest/testdata/logic_test/interval, line 463 at r1 (raw file):

  iso,
  sql_std,
  default_std,

could also add a check for making sure pg == default_std

pkg/sql/sem/builtins/builtins.go, line 2358 at r1 (raw file):

				return tree.NewDString(buf.String()), nil
			},
			Info:       "Convert an interval to a string assuming the Postgres IntervalStyle.",

hm, the name to_char_with_style might be a bit confusing given that it doesn't actually take a style param. is it better to name it to_char?

pkg/sql/sem/builtins/builtins.go, line 2391 at r2 (raw file):

				return tree.NewDString(tree.AsStringWithFlags(&ts, tree.FmtBareStrings)), nil
			},
			Info:       "Convert an timestamp to a string assuming the ISO, MDY DateStyle.",

similar to other comment: to_char_with_style without a style parameter might be confusing

pkg/sql/sem/builtins/builtins.go, line 2394 at r2 (raw file):

			Volatility: tree.VolatilityImmutable,
		},
		tree.Overload{

should all these two-arg overloads use stringOverload2()?

pkg/sql/sem/builtins/builtins.go, line 2419 at r3 (raw file):

				return tree.NewDString(tree.AsStringWithFlags(&ts, tree.FmtBareStrings)), nil
			},
			Info:       "Convert an date to a string assuming the ISO, MDY DateStyle.",

similar: to_char_with_style without a style param might be confusing

pkg/sql/sem/tree/datum.go, line 1865 at r3 (raw file):

// be used instead of direct type assertions wherever a *DDate wrapped by a
// *DOidWrapper is possible.
func AsDDate(e Expr) (DDate, bool) {

where is AsDDate used, and also when would *DDate be wrapped by DOidWrapper?

Release note (sql change): Introduce parse_interval and to_char which takes in 1 string or interval and assumes the Postgres IntervalStyle to make it's output.

Release note (sql change): parse_timestamp now has a two argument variant, which takes in a DateStyle and parses timestamps according to that DateStyle. The one argument version assumes MDY. These builtins have an immutable volatility. Release note (sql change): Introduce a timestamp,DateStyle variant to to_char_with_style, which converts timestamps to a string with an immutable volatility. There is also a 1 arg to_char for timestamp values which assumes the ISO,MDY output style.

Release note (sql change): Introduce a parse_date builtin with two variants - the single arg variants parses a date and assumes ISO,MDY datestyle and the two arg variants parses a date assuming the datestyle variant on the second argument. This provides an immutable way of casting strings to dates. Release note (sql change): Introduce to_char(date), which assumes a DateStyle of ISO,MDY and outputs date in that format. There is also a to_char_with_style(date, DateStyle) variant which outputs the date in the chosen DateStyle. This provides an immutable way of casting dates to strings.

Note to_char_with_style is not needed as these types do not need date to output. Release note (sql change): Implement parse_time and parse_timetz builtins, which parses a time or timetz with immutable volatility.

rafiss

lgtm! though maybe you have some unpublished reviewable comments?

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan)

otan

ah oops

Dismissed @rafiss from a discussion.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan and @rafiss)

pkg/sql/logictest/testdata/logic_test/datetime, line 1864 at r2 (raw file):