sql: fix sub-second EXTRACT behaves differently from postgres

[hueypark]

Fixes #40683

Release note (sql change): sub-second EXTRACT behaves same as postgres.

Release justification: low-risk bugfix.

[cockroach-teamcity]

This change is 

[maddyblue]

Code LGTM. @jordanlewis will make a decision about whether we will ship this in 19.2

[knz]

IMHO this is a bug fix, not "sql change", and is even candidate to back port in 19.1.

[maddyblue]

It's a bug fix only in that we differ from the spec: it was acting exactly as we intended otherwise. Anyone relying on EXTRACT will see a massive behavior change that will break their app. For that reason I think we should definitely not backport it to 19.1. 19.2 is a possibility because we haven't shipped that yet and so we allow some behavior changes. I'm still lightly on the "it should wait for 20.1" side myself.

[maddyblue]

As another example, our int / int operator differs from postgres. If we changed that today to exactly match postgres it seems unlikely we would backport that to 19.1, and it's even weird to consider shipping that in 19.2 at this point in the release cycle.

[maddyblue]

We've decided to wait for 20.1 to ship this. I'll merge this as soon as the master branch is open, which we expect to be in a week.

[hueypark]

Thanks for the careful review!

[maddyblue]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)

[jseldess]

@mjibson, @knz, shouldn't this be listed as a backward-incompatible change, given what Matt says above?

Anyone relying on EXTRACT will see a massive behavior change that will break their app.

[knz]

yes I would agree with that (but the phrasing of the change should still emphasize that the previous crdb behavior was unintended — i.e. we consider it as a bug)

[jseldess]

@mjibson, can you please write a sentence or two about how "anyone relying on EXTRACT will see a massive behavior change"? If we call this out as backward-incompatible, we need to say more.

[maddyblue]

EXTRACT with sub-second arguments (millisecond, microsecond) is now Postgres-compatible, and will return the total number of seconds in addition to sub seconds. Anyone who was previously relying on EXTRACT to return only sub-second data will need to adjust their applications.

Is that good?

[jseldess]

Yep. Thanks, Matt.