Add clickhouse support #264
Conversation
dbr_test.go
Outdated
| @@ -100,73 +131,78 @@ func reset(t *testing.T, sess *Session) { | |||
|
|
|||
| func TestBasicCRUD(t *testing.T) { | |||
| for _, sess := range testSession { | |||
| reset(t, sess) | |||
There was a problem hiding this comment.
There's no big change here, I just changed the test name to include the dialect, otherwise it's difficult to see which one was failing.
There was a problem hiding this comment.
Changed all tests to be this way in https://github.com/gocraft/dbr/pull/268/files
.circleci/config.yml
Outdated
| @@ -2,11 +2,12 @@ version: 2 | |||
| jobs: | |||
| build: | |||
| docker: | |||
| - image: circleci/golang:1.17 | |||
| - image: cimg/go:1.21 | |||
There was a problem hiding this comment.
As per https://hub.docker.com/r/cimg/go circleci/golang was deprecated in favor of cimg/go
|
@cypriss @taylorchu if you feel the PR ended up being too big, I'm happy to split it in a few, to make it easier to review, let me know |
| mysql | ||
| } | ||
|
|
||
| func (d clickhouse) EncodeTime(t time.Time) string { |
There was a problem hiding this comment.
All the dialects have a received named d. I wanted to preserve the same naming
join.go
Outdated
| @@ -4,15 +4,21 @@ type joinType uint8 | |||
|
|
|||
| const ( | |||
| inner joinType = iota | |||
| allFull | |||
There was a problem hiding this comment.
is this going to affect something? because the iota for left, right and full will change.
There was a problem hiding this comment.
These are only used internally and are kind of enums to know which string to write, so shouldn't affect
| @@ -13,4 +13,11 @@ type Dialect interface { | |||
| EncodeBytes(b []byte) string | |||
|
|
|||
| Placeholder(n int) string | |||
|
|
|||
| UpdateStmts() (string, string) | |||
There was a problem hiding this comment.
could you comment on why each is added?
There was a problem hiding this comment.
UpdateStmtswas added to know what is the staement that has to be used to run UPDATEs. Unfortunatelly clickhosue is different in this part, it's in the shape ofALTER TABLE ... UPDATE key = valuecompared to others where it'sUPDATE ... SET key = valueOnConflictadds support to add actions for constraint violation, e.g UPSERT. This works in postgres, mysql, etc. I can maybe add this change in a separate PR if you think it would be better to review, let me know if you want me to split this into smaller PRs, happy to do.Proposedis used with OnConflict as reference to proposed value in on conflict clause. You can see the TestOnConflict to see how it works.SupportsOnwas added for clickhouse explicitly, but I removed it for now, I can maybe add it later if we really need to use it, and I think it should be done in a different way.
There was a problem hiding this comment.
Created a separate PR for the CI updates #265 I can then split this into more pieces if that's ok.
|
@taylorchu I removed things that were not specific to clickhouse from this PR and created: which are ready to be reviewed. Let me know your thoughts! 🙌 |
cc @cypriss / @taylorchu