erleans / pgo

Erlang Postgres client and connection pool
Apache License 2.0
80 stars 16 forks source link

function_clause error when querying if tstzrange contains timestamptz #36

Open benbro opened 4 years ago

benbro commented 4 years ago

This gives an error:

pgo:query(<<"SELECT tstzrange('2020-01-01 00:00:00+00', '2021-01-01 00:00:00+00', '[)') @> $1;">>, [{{2020, 2, 1}, {0, 0, 0}}]).
{error,function_clause}

Casting to timestamptz works:

pgo:query(<<"SELECT tstzrange('2020-01-01 00:00:00+00', '2021-01-01 00:00:00+00', '[)') @> $1::timestamptz;">>, [{{2020, 2, 1}, {0, 0, 0}}]).
#{command => select,num_rows => 1,rows => [{true}]}
tsloughter commented 4 years ago

Oh, I missed that you opened this.

I don't think there is a fix here besides casting. Well, that isn't fair, the "fix" is that pgo needs to return a real error message. But I don't think it can infer the type. Let me figure out fixing error messages and we can see what it is actually getting wrong and if there might be a fix for this besides casting.

benbro commented 4 years ago

Doesn't it work automatically in epgsql?

tsloughter commented 4 years ago

Possibly but it might rely on the text protocol for that.

With the binary protocol, which pgo uses, it needs to know the type upfront and it may not be able to figure it out without the cast. But I need to fix the error message to be sure.

tsloughter commented 4 years ago

Yea, so I'm about to push an update to pgo that gives better errors and optional formated error messages. And for your original query I get:

Error encoding type tstzrange. Expected, empty | {{From::unbound | term(), To::unbound | term()}, {LowerInclusive::boolean(), UpperInclusive::boolean()}}. Got, {{2020, 2,  1}, {0, 0, 0}}.

So it expects a tstzrange but you are giving it a timestamp, that is why the cast is needed. Again, I'm guessing in epgsql, if that query without casting works, they may be using the text protocol here to get around this issue but I can't be certain.

tsloughter commented 4 years ago

Hm, or is tstzrange supposed to support being passed timestamps?

tsloughter commented 2 years ago

Looking at this again and based on my last comment I'm guessing I realized you were right and it should just be accepting what you pass :). It would mean updating https://github.com/tsloughter/pg_types/blob/master/src/pg_range.erl to accept a single term.

tsloughter commented 2 years ago

Nevermind, I found the bug.

The issue is pg_range matches on {From, To} and a timestamp is also a 2-tuple.

I'm not sure the best way to get around this issue yet, but I found it knows it is needing to encode the value with pg_timestampz, so it is just a matter of knowing it isn't a {From, To} range, but instead a timestamp.

It may have to be an ugly hack of matching {{_,_,_},{_,_,_}} first, but since pg_range isn't specific to timestamps that is a pretty bad way around this.

I suppose the other option would be to not support turning {From, To} into {{From, To}, {true, true}} automatically. That would break the API but probably no one is using it anyway...

Best solution would be to have a timestamp record or map, but that'd be as much of a pain.

benbro commented 2 years ago

Requiring {{From, To}, {true, true}} instead of {From, To} might be fine.

tsloughter commented 2 years ago

I'm still trying to figure out what it is Postgres expects for the encoding of this. Since there is no damn spec I'm trying to read the epgsql and postgrex code but it isn't yet clear what they do for a case like this -- trying to even find a test case in postgrex.