sql: Add precision to timestamp objects, with 6 as the default precision value

[rohany]

This PR implements support for TIMESTAMP and TIMESTAMPTZ objects to be given a precision when created, for example TIMESTAMP(0). TIMESTAMP objects have precision 6 by default, and only the precision values 0 and 6 are supported right now.

[cockroach-teamcity]

This change is 

[solongordon]

Looks like a good start. BTW, this was not explicit in the issue but we want to support 0 precision for TIMESTAMPTZ and TIME as well.

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

[rohany]

Ok, this is ready for review -- I'll squash the commits and update the messages once it is ready for landing.

[solongordon]

Added a testing suggestion. I'm still puzzling through some of the types stuff.

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

---

pkg/sql/logictest/testdata/logic_test/timestamp, line 20 at r8 (raw file):

statement ok
CREATE TABLE timestamp_test (x TIMESTAMP(0)); INSERT INTO timestamp_test VALUES ('1-1-18 1:00:00.001'), ('1-1-18 1:00:00.00');

Generally I think we try not to have multiple statements per line in these tests, just to keep things readable.

Actually creating a table seems unnecessary here. You can just do SELECT '1-1-18 1:00:00.001':::TIMESTAMP(0) and verify the result. (Same for all the following tests.)

[solongordon]

The rest LGTM with some nits. The whole default -1 thing bugs me a little bit but I can't think of a great alternative.

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

---

pkg/sql/types/types.go, line 1090 at r8 (raw file):

		return "JSONB"
	case TimestampFamily, TimestampTZFamily:
		// This is default timestamp

Maybe clearer to say it's a timestamp with the default precision.

---

pkg/sql/types/types.go, line 1094 at r8 (raw file):

			return strings.ToUpper(t.Name())
		}
		return fmt.Sprintf("%s(%d)", strings.ToUpper(t.Name()), t.Precision())

Nit: Slightly simpler would be to just do:

if t.Precision() != -1 {
  return fmt.Sprintf("%s(%d)", strings.ToUpper(t.Name()), t.Precision())
}

and just rely on the default case for when precision == -1.

---

pkg/sql/types/types.go, line 1097 at r8 (raw file):

	case TimeFamily:
		if t.Precision() > 0 {
			return fmt.Sprintf("TIME(%d)", t.Precision())

Nit: Might as well continue using strings.ToUpper(t.Name()) here rather than hard-coding TIME.

[rohany]

I don't like the -1 default either, but it is tricky for printing purposes to know whether the timestamp precision is "default 6" or manually set to 6

[solongordon]

The alternative would be to add a boolean field specifying if Precision is set, but not sure that's an improvement in this case.

[rohany]

yeah, i would prefer to avoid changing protos and use elsewhere of the type

[solongordon]

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

[rohany]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)

[jordanlewis]

@rohany this is great! Could you please update the PR title and body to match the commit, to make it more obvious that this PR is about more than just 0 precision timestamp objects (which I think it is)?

[rohany]

@jordanlewis is that good, or do you have something else in mind?

[jordanlewis]

The title's good - can you update the body too? (like the first message in the PR). You can just edit it to be the same as your commit message. The reason it's important is to make sure that when someone inevitably goes back to this PR to try to figure out what the change was, they'll be able to understand the final merged state of the PR just by reading the subject.

[rohany]

Ok great -- sorry, I didn't know what you meant by update the body!

[jordanlewis]

Perfect - thanks!

[awoods187]

This pr was for this issue: #32098

[awoods187]

Also related to #16349