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 Support #374

Closed
mholt opened this issue Apr 3, 2018 · 15 comments

Comments

Projects
None yet
4 participants
@mholt
Copy link

commented Apr 3, 2018

Not a bug report, but a feature request to support CockroachDB. Its admin interface doesn't support viewing tables and running queries, so it would be great to see that in TablePlus.

@huyphams

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2018

CockroachDB uses PostgreSQL protocol to connect to its database. I will optimize it in the future (it's already on our roadmap).

@mholt

This comment has been minimized.

Copy link
Author

commented Apr 3, 2018

Great! Yeah, it just seems to work somewhat but when I go to list the contents of a table, it says ERROR: column name "ctid" not found -- that's the only error I have seen so far. Hopefully it won't take too long to do since you already have fantastic support for PostgreSQL. Thanks again! Looking forward to switching from Postico.

@huyphams

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2018

Hey @mholt I just added support for Cockroach DB (maybe there are some bugs) can you give it a try?

Download: https://www.dropbox.com/s/fghht69ib70jdpn/TablePlus.zip?dl=0

screen shot 2018-04-03 at 11 10 43 pm

@mholt

This comment has been minimized.

Copy link
Author

commented Apr 3, 2018

@huyphams Hey, this is great, from my basic testing it works! Wow. You're fast. That means you probably have a pretty good underlying design. Nicely done.

@codingconcepts

This comment has been minimized.

Copy link

commented Apr 3, 2018

@huyphams, thanks for adding CockroachDB support!

I've just had a little play with it myself and would second @mholt on the design sentiment!

One thing I've noticed that might be quite useful, would be to limit the available data types in the create table data_type dropdowns, so that it only contains those supported by CockroachDB (https://www.cockroachlabs.com/docs/stable/data-types.html). It's possible to add string via manual entry but it'd be great to be able to select it :)

Thanks again!

@codingconcepts

This comment has been minimized.

Copy link

commented Apr 3, 2018

I'm not sure if this is a CockroachDB-specific error, but I'm trying to change the string array value {"a","b","c"} to {"a","b","d"} and receive the following:

image

It looks like the cell value is being interpreted as a string and not parsed as a string[] during commit because the column's type isn't being inferred correctly. Here are some other array columns with the same issue:

image

@huyphams

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2018

Hey thanks, I added CockroachDB yesterday but didn't check all the case. I will do it today to cover all the syntax.

@huyphams

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2018

This is the new build, it fixes many issues: https://www.dropbox.com/s/fghht69ib70jdpn/TablePlus.zip?dl=0
Thanks guys for testing this Cockroach DB driver.

Remaining:

  • Auto convert '{}' syntax to Array[].
  • Support data length in structure(example: STRING(20))
  • Support create seq.
@codingconcepts

This comment has been minimized.

Copy link

commented Apr 4, 2018

This could very well be me being stupid but this is appears to be the same file (shasum wise)!

Is this definitely the new version, @huyphams?

Cheers!

Rob

@huyphams

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2018

Hi @codingconcepts it's new build. I just updated the file on Dropbox (don't need to change the link).

@mholt

This comment has been minimized.

Copy link
Author

commented Apr 5, 2018

I didn't compute a checksum but I can confirm it is different / improved.

Still needs a little work, but I like how it's coming along. I'll try to scrounge up some time to give more.specific feedback later.

@huyphams

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2018

Hey, I think everybody here is using Cockroach every day so I distribute this build (because waiting is a pain):

https://www.dropbox.com/s/fghht69ib70jdpn/TablePlus.zip?dl=0 (till the same URL).

It's usable in common cases:

  • Query.
  • Update, delete, insert. (Now it supports transform array from {} to Array automatically).
  • And all the other basic functions.

Enjoy and welcome any feedback 👍

@glerchundi

This comment has been minimized.

Copy link

commented Apr 5, 2018

@huyphams fantastic job!

I didn't test it out extensively but looks promising, will send a feedback.

Just one nit, there is a discrepancy between which extension CockroachDB expects for certificates and TablePlus allows to pick. CockroachDB looks for (when using their cli with --certs-dir) for *.crt extensions (concretely for ca.crt and client.<user>.crt) but TablePlus requires these to be ending in .pem. If its possible it would ease our lives a little bit if TablePlus allowed the selection of *.crt files for certificates as well.

@huyphams

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2018

Sure @glerchundi 👍

@huyphams huyphams added done and removed work-in-progress labels Apr 11, 2018

@huyphams huyphams closed this Apr 11, 2018

@mholt

This comment has been minimized.

Copy link
Author

commented Apr 11, 2018

I don't know if it matters, but something to note is that .pem and .crt usually imply different encodings (at least in web server land): .crt is usually DER-encoded and .pem is of course PEM-encoded. That would make a difference when reading the file. If you only support one format, document that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.