x/cqlbuilder: Add implementation of a query builder #2
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
x/cqlbuilder/predicate.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func clonePredicateClause(pc PredicateClause) PredicateClause { |
There was a problem hiding this comment.
[golangci-linter] reported by reviewdog 🐶
func clonePredicateClause is unused (unused)
x/cqlbuilder/dml.go
Outdated
| } | ||
|
|
||
| func (pcl PredicateLWTClause) keys() []string { | ||
| var ks []string |
There was a problem hiding this comment.
[golangci-linter] reported by reviewdog 🐶
Consider preallocating ks (prealloc)
| }) | ||
| } | ||
|
|
||
| func queryRow(sq *SelectQueryer, foo string) (string, error) { |
There was a problem hiding this comment.
[golangci-linter] reported by reviewdog 🐶
queryRow - foo always receives "foo" (unparam)
| ) | ||
| } | ||
|
|
||
| func execCAS(e Execer, foo, bar string) (bool, string, string, error) { |
There was a problem hiding this comment.
[golangci-linter] reported by reviewdog 🐶
execCAS - foo always receives "foo" (unparam)
| fmt.Fprintf(w, " TTL %d", int(do.TTL.Seconds())) | ||
|
|
||
| if !do.Timestamp.IsZero() { | ||
| io.WriteString(w, " AND") |
There was a problem hiding this comment.
[golangci-linter] reported by reviewdog 🐶
Error return value of io.WriteString is not checked (errcheck)
| } | ||
|
|
||
| if v.Len() == 0 { | ||
| io.WriteString(qw, "1=0") |
There was a problem hiding this comment.
[golangci-linter] reported by reviewdog 🐶
Error return value of io.WriteString is not checked (errcheck)
| @@ -32,6 +32,8 @@ var defaultOptions = options{ | |||
|
|
|||
| type Option func(*options) | |||
|
|
|||
| func MigrationTable(t string) Option { return func(o *options) { o.table = t } } | |||
There was a problem hiding this comment.
[golangci-linter] reported by reviewdog 🐶
func name will be used as migration.MigrationTable by other packages, and that stutters; consider calling this Table (golint)
x/cqlbuilder/dml.go
Outdated
| return plc.Predicate.WriteTo(qw, vs) | ||
| } | ||
|
|
||
| func (pcl PredicateLWTClause) keys() []string { |
There was a problem hiding this comment.
[golangci-linter] reported by reviewdog 🐶
receiver name pcl should be consistent with previous receiver name plc for PredicateLWTClause (golint)
x/cqlbuilder/dml.go
Outdated
| } | ||
|
|
||
| func (PredicateLWTClause) isUpdateClause() {} | ||
| func (PredicateLWTClause) isDeleteClause() {} |
There was a problem hiding this comment.
[golangci-linter] reported by reviewdog 🐶
ST1016: methods on the same type should have the same receiver name (seen 1x "pcl", 1x "plc") (stylecheck)
AlexisMontagne
force-pushed
the
am-cqlbuider
branch
from
27d91f6
to
17cde24
Compare
x/cqlbuilder/query_builder.go
Outdated
|
|
||
| import ( | ||
| "github.com/upfluence/cql" | ||
| ) |
There was a problem hiding this comment.
| ) | |
| import "github.com/upfluence/cql" |
:p
|
Otherwise, my very small comments, LGTM |
upfluence-bot
added
reviewed/ready-to-merge
reviewed/needs-review
and removed
reviewed/needs-work
reviewed/ready-to-merge
labels
AlexisMontagne
force-pushed
the
am-cqlbuider
branch
from
7a15ae4
to
5445a2f
Compare
AlexisMontagne
force-pushed
the
am-cqlbuider
branch
from
5445a2f
to
4abec7a
Compare
| ExecCAS(context.Context, map[string]interface{}) CASScanner | ||
| } | ||
|
|
||
| type execer struct { |
There was a problem hiding this comment.
Cassandra noob here,
What's the difference between Exec and ExecCAS ? Expecting a scanner to deal with some output ? If so what does CAS stands for ?
| casScanKeys() []string | ||
| } | ||
|
|
||
| type InsertExecer struct { |
There was a problem hiding this comment.
I'm not sure to understand the point of those Insert|Update|Delete execers ? Why not returning an execer directly ?
| }) | ||
| } | ||
|
|
||
| func TestEC(t *testing.T) { |
There was a problem hiding this comment.
What does EC stands for ? What does this test tests ?
| } | ||
|
|
||
| func wrapMultiClause(wcs []PredicateClause, op string) PredicateClause { | ||
| var cs []PredicateClause |
There was a problem hiding this comment.
[golangci-linter] reported by reviewdog 🐶
Consider preallocating cs (prealloc)
| } | ||
|
|
||
| func (mc multiClause) Markers() []Marker { | ||
| var ms []Marker |
There was a problem hiding this comment.
[golangci-linter] reported by reviewdog 🐶
Consider preallocating ms (prealloc)
What does this PR do?
C* is the second most used database at Upfluence, from the experience learnt of using upfluence/sql/x/sqlbuilder the idea is to provide similar DSL to create easy to build / composable request to a cassandra cluster.
What are the observable changes?
Too much to mention here.
Good PR checklist
Additional Notes