colexec, coldata: add support for INTERVAL type

[yuzefovich]

pgerror: clean up build deps

The pgerror (and pgcode) packages are (perhaps inadvisably) used in
low-level utility packages. They had some pretty heavyweight build deps,
but this wasn't fundamentally necessary. Clean it up a bit and make
these packages more lightweight.

Release note: None

colexec, coldata: add support for INTERVAL type

This commit adds the support for INTERVAL type that is represented by
duration.Duration. Only comparison projections are currently supported.
The serialization is also missing.

Addresses: #42043.

Release note: None

[cockroach-teamcity]

This change is 

[asubiotto]

Do we have an issue that tracks types that need serialization/more efficient serialization? We should also track the need to support other types of operators for certain types, including interval.

duration.Duration doesn't lend itself to efficient serialization, but I'm not sure what to do about it right now. We might need to write an abstraction layer on top of certain types that have inefficient serialization at some point. This might be something equivalent to flat bytes where we represent []duration.Duration as a slice of int64s that are interpreted into durations three by three.

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

[yuzefovich]

@danhhz are you ok with the second commit here which whitelists many dependencies that util/duration brings into pkg/workload?

[yuzefovich]

Friendly ping @danhhz

[danhhz]

Woah, that's a lot of stuff! No we probably shouldn't do that. Have you looked into why it brings so much in?

[danhhz]

It looks like the issue is the errEncodeOverflow error returned from Duration.Encode. I haven't looked closely, but it may be an easy fix for that method to return a bool (true for overflow) instead of an error, and turn that into a errEncodeOverflow at the callers

[yuzefovich]

Thanks for the comments, I'll take a deeper look.

[danhhz]

So I tried prototyping this and quickly found out that this error ends up getting plumbed back all the way even through the KV api (via internal/client/util.marshalValue). I think the issue here is that the pgcode error wrapper, while useful context for the user, has been sprinkled in utility libraries that it probably doesn't belong in. All of this is not your code, I understand. Hmmm....

To be honest, how much do we lose by simply removing the pgcode wrapper from this one place? That certainly seems like the easiest fix. We still get the error message for the user and I somewhat doubt any clients are actually sniffing this code to do something useful

[danhhz]

The other way we could go at this is to make pgerror a lighter dep.

• The real issue seems to be its use of github.com/cockroachdb/cockroach/pkg/util/log, which is only included for DecodeStackTrace and EncodeStackTrace, both of which only have stdlib and external deps. So we could move those two somewhere else.
• The other one seems to be the decodeWithCandidateCode method, which has an argument of type "protoutil.SimpleMessage". This appears to be the case only because having "proto.Message" would be a linter error, so maybe we can disable that one lint here

[danhhz]

I'm leaning toward the last one. It will be functionally identical. Plus, while in theory I think putting these pgerrors in util packages is unfortunate, I must admit that practically it's not worth it to fix them at this point. Cleaning up pgerror lets us sidestep the issue

[danhhz]

Ugh, just noticed that DecodeStackTrace uses log.Errorf

[danhhz]

Okay, I think I made this work. @yuzefovich would you mind cherry-picking this into your PR? https://github.com/cockroachdb/cockroach/compare/master...danhhz:pgerror_deps?expand=1

[yuzefovich]

Yeah, that seems to do the trick, thanks!

[yuzefovich]

Updated, PTAL.

[danhhz]

LGTM

[yuzefovich]

I needed to make a few tweaks to the commit that cleans up the dependencies, please take another look.

[yuzefovich]

@danhhz could you please take another quick look that the changes I made to make the linters happy are acceptable.

[danhhz]

Whoops sorry! This got lost in my inbox. LGTM

[yuzefovich]

TFTRs!

Do we have an issue that tracks types that need serialization/more efficient serialization?

Yep, filed #44195.

We should also track the need to support other types of operators for certain types, including interval.

So far (I think) the only missing projections are of combination of Timestamps and Intervals. There is an issue for that #42044.

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)