pgwire: only show TZ minutes offset if non-zero

[rafiss]

Similar to #57265, this commit matches the PG behavior to
only show the minutes offset if it is non-zero.

Release note (bug fix): Minute timezone offsets are only displayed in
the wire protocol if they are non-zero for TimestampTZ and TimeTZ values.
Previously, they would always display.

Release note (bug fix): Binary TimeTZ values were not being decoded
correctly when being sent as a parameter in the wire protocol. This
is now fixed.

Release note (cli change): The SQL shell now formats times with time
zones so that the minutes and seconds offsets are only shown if they are
non-zero. Also, infinite floating point values are formatted as
Infinity rather than Inf now.

[cockroach-teamcity]

This change is 

[rafiss]

Next question -- should we also update the timestamptz->string and timetz->string casts so that they only show the minutes offset if non-zero? That is the Postgres behavior and also that's how we treat the seconds offset already. Downside is that maybe it's too backwards-incompatible. @otan curious on your thoughts

[otan]

yeah backwards compatibility is hard - you could do this with a cluster setting, off by default in v21.1 -> v21.2 but on by default for new v21.2 clusters.

the change would have to be made here:

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

Line 2569 in 695a2d6

ctx.WriteString(d.Time.Format(timestampTZOutputFormat))

does this work correctly on the CLI side? I think you also need to do something with lib/pq a la #62310

[otan]

change as is LGTM

[rafiss]

yeah backwards compatibility is hard - you could do this with a cluster setting, off by default in v21.1 -> v21.2 but on by default for new v21.2 clusters.
the change would have to be made here:

yeah i already had the change ready to go, but just wasn't sure about compatibility. i'll look into the cluster setting idea in a different PR -- is there already a thing we do to control default values for upgrades versus new clusters?

does this work correctly on the CLI side? I think you also need to do something with lib/pq a la #62310

it doesn't show correctly in CLI, i was wondering about that. thanks for the pointer, i will add that to this PR

[otan]

yeah i already had the change ready to go, but just wasn't sure about compatibility. i'll look into the cluster setting idea in a different PR -- is there already a thing we do to control default values for upgrades versus new clusters?

i don't know unfortunately. it's tricky to fix, and i think why i avoided it.

[rafiss]

RFAL on the cli change. i made it also account for #64760

[rafiss]

tftr!

bors r=otan

[craig]

Build failed:

• GitHub CI (Cockroach)

[rafiss]

bors r=otan

[craig]

Build succeeded:

• GitHub CI (Cockroach)