Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CockroachDB supports the PostgreSQL protocol but connection fails due to "IntervalStyle": "iso_8601" #130

Closed
bmisiak opened this issue Mar 17, 2020 · 3 comments · Fixed by #135

Comments

@bmisiak
Copy link
Contributor

bmisiak commented Mar 17, 2020

Hi.

I tried connecting to CockroachDB just now and hit a little snafu. It supports the PostgreSQL protocol, but it apparently requires IntervalStyle to be set to postgres instead of iso_8601.

SQLx sets it to the latter here:

("IntervalStyle", "iso_8601"),

Which results in:

Error: invalid value for parameter "IntervalStyle": "iso_8601"

Is supporting postgres here easy for SQLx, or would it be a significant headache? I was worried about INTERVAL fields but they seem to be returned to the consumer as strings, since conversion to Duration impls haven't been written yet.

When I naively changed the field to postgres in my fork, connection succeeded and queries work.

@abonander
Copy link
Collaborator

abonander commented Mar 17, 2020

Yeah, that really only matters if we actually support encoding and decoding intervals which we don't currently. Neither chrono nor time support parsing a Duration type, which would be the Rust equivalent. We could manually implement it using the Postgres style but that's a bit involved.

@mehcode is there another reason you have IntervalStyle set?

@mehcode
Copy link
Member

mehcode commented Mar 17, 2020

I'd accept just removing the explicit set of IntervalStyle. My plan was to add a PgInterval type but we can (and probably) should add parsing for "postgres" format instead of the newer "iso8601" format.


I'm not sure I'm comfortable claiming to support CockroachDB. If we can make this change and it works 100%, great, but we probably need to enumerate a "Database Support" chart like Rust does for platforms (perhaps on the Readme). We could have "Tier 1" being guaranteed and "Tier 2" being "someone told us this works". I'd mark CockroachDB under "Tier 2".

@bmisiak
Copy link
Contributor Author

bmisiak commented Mar 18, 2020

@mehcode For sure, agreed. I have tested it and it seems to be working beautifully with SQLx after removing IntervalStyle.

.. as well as other ordinary PostgreSQL clients:

image

image

I'll submit a PR to remove it. It's only a SET SESSION IntervalStyle = 'iso_8601' away after all.

bmisiak added a commit to bmisiak/sqlx that referenced this issue Mar 18, 2020
Support more server versions, including CockroachDB, which only supports the `postgres` style.
mehcode added a commit that referenced this issue Mar 18, 2020
postgres: remove IntervalStyle to fix #130
janaakhterov pushed a commit that referenced this issue Mar 21, 2020
Support more server versions, including CockroachDB, which only supports the `postgres` style.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants