sql/sem/tree: parse ISO8601 format with decimal places for interval types

[keithdoggett]

We do not parse SELECT INTERVAL 'P1Y2M3DT4H5M6.235S'; correctly on CockroachDB, but this successfully parses in Postgres:

psql (13.1)
Type "help" for help.

otan=# SELECT INTERVAL 'P1Y2M3DT4H5M6.235S';
             interval
-----------------------------------
 1 year 2 mons 3 days 04:05:06.235
(1 row)

Changes should be made in

cockroach/pkg/sql/sem/tree/interval.go

Line 188 in 3699ce6

func sqlStdToDuration(s string, itm types.IntervalTypeMetadata) (duration.Duration, error) {

With unit tests added here:

cockroach/pkg/sql/sem/tree/interval_test.go

Line 46 in 3699ce6

{`1`, types.IntervalTypeMetadata{}, `00:00:01`},

---

Original:

There are two errors coming from the PostgresqlIntervalTest (test/cases/adapters/postgesql/interval_test.rb) in ActiveRecord.

Error:
PostgresqlIntervalTest#test_interval_type:
ActiveRecord::StatementInvalid: PG::SyntaxError: ERROR:  error in argument for $2: could not parse string "P1Y2M3DT4H5M6.235S" as interval

Error:
PostgresqlIntervalTest#test_interval_type_cast_from_numeric:
ActiveRecord::StatementInvalid: PG::SyntaxError: ERROR:  error in argument for $1: could not parse string "PT36000.000S" as interval

Running SELECT INTERVAL 'P1Y2M3DT4H5M6.235S'; in a SQL shell gives the following response:

ERROR: could not parse "P1Y2M3DT4H5M6.235S" as type interval: interval: unknown unit . in ISO-8601 duration P1Y2M3DT4H5M6.235S

This indicates that using a decimal for seconds is causing the error.

[rafiss]

Looks like https://github.com/senseyeio/duration/blob/7c2a214ada46/duration.go#L31 is a reasonable (and MIT license) reference we could use. just a simple regex. (we should reimplement ourselves since there's no need to add a dependency for something so small)

[shikamaru-cc]

hi @keithdoggett @rafiss
im new to the community
this seems so great to be my first issue!
If nobody is working on it, I would like to try

[rafiss]

@shikamaru404 welcome! thank you for your interest! please go ahead and try it out. when you submit a PR, please add @otan and me as reviewers.

[shikamaru-cc]

hi @rafiss
Should the changes be in function sqlStdToDuration (cockroach/pkg/sql/sem/tree/interval.go)?
Seems that function iso8601ToDuration (cockroach/pkg/sql/sem/tree/interval.go) is responsible for parsing iso8601.

[otan]

@shikamaru404 seems like you're right :)