sql: Timestamps don't work through pgwire

[spencerkimball]

"unsupported OID 1114" error

The code contains this nugget in sql/pgwire/types.go:

    // TODO(mjibson): implement date/time types
    default:
        return d, fmt.Errorf("unsupported OID: %v", id)

[tamird]

I've looked into this - it's going to require some timestamp parsing code which might be provided by #4858. Further, much of this code is already in lib/pq but not exported; I'll look into exporting it upstream.

[petermattis]

@tamird I think @paperstreet has more bandwidth than you right now.

[tamird]

SGTM

On Thu, Mar 10, 2016 at 4:19 PM, Peter Mattis notifications@github.com
wrote:

@tamird https://github.com/tamird I think @paperstreet
https://github.com/paperstreet has more bandwidth than you right now.

—
Reply to this email directly or view it on GitHub
#4872 (comment)
.

[danhhz]

Okay, so I've looked into this a bit. #4858 has interval parsing but not timestamps. Our lib/pq fork has an unexported function to do this, but it panics on any error. I could wrap that in a deferred recover and call it a day or I could reimplement parseTs with actual error handling. I'm leaning toward the latter but before I actually did the work, I figured I'd get a second opinion. Thoughts @tamird or @petermattis? FWIW, I benchmarked how much overhead the defer version is:

BenchmarkParseTs-8            100000         16556 ns/op         928 B/op         29 allocs/op
BenchmarkParseTsRecover-8     100000         19341 ns/op         928 B/op         29 allocs/op

[tamird]

@paperstreet I'd suggest changing the function in our fork to (1) export it and (2) give it real error handling.

We should attempt to push upstream as a follow-up task. Sound reasonable?

[danhhz]

Great. My thoughts exactly

[danhhz]

Oops, misunderstood was spencer was saying on the review and didn't fix my issue reference.