Hi Thomas,
I’ve thought about writing something like this. My postgreSQL queries go
through database/sql and github.com/lib/pq and are concatenated strings
with + and constants in most cases, and sometimes with Sprintf to write an
index naming a column. This approach is hard to read. I have this issue
about that: https://github.com/pciet/wichess/issues/49. It looks like your
team has a lot more experience with SQL than me.
One thought is that the SQL seems to differ between relational database
implementations. I’ve argued that database/sql should be a helper library
instead of a gateway library because of this. Perhaps
github.com/ulule/loukoum could be a way to hide those database differences
from query construction? The goal would be to make servers using SQL
relational database independent.
Anyway, here’s a code review.
I haven’t seen a library that exposes the API this way, with one file
importing the sub-packages. While this seems clean I’ve been arguing for
more use of flat packages in cases like this. For example, “type Map =
types.Map” would be unnecessary if there was no types package. The
functions here also require knowledge of the sub-packages anyway since they
return types from those packages.
I prefer package names with a hint to what they do. The meaning of
loukoum.Pair(…) can’t be inferred. Something like sqlb.Pair(…) would be my
choice.
“func ToColumns(values []interface{})” isn’t clear to me. From the
implementation it can either be a slice with one element of []stmt.Column
or []string, or a slice of stmt.Column or string. I’ve been suggesting
adding a type for interface{} in cases like this that documents what the
expected types are:
// A Column can be string or stmt.Column.
type Column interface{}
I’m only saying this because these are exported functions of
github.com/ulule/loukoum/builder, if they were private functions then the
code may be documentation enough.
To me this shows that something might be improved to avoid this need in the
library code:
var _ Builder = Delete{}
Along with that, I’m not sure about the Builder interface. If the interface
is not part of a function call in the same file or package I’m worried.
I’ve previously suggested grouping behavior as a struct type of function
fields instead of relying on a type hierarchy, or there may be other ways
to do it, but an interface should have an obvious use locally. Maybe having
the flat package would make this interface more clear.
Again, the “type Select struct { query stmt.Select }” seems like a symptom
of package overuse.
In lexer:
l := Lexer{}
l.input = buffer
could instead be
l := Lexer{input: buffer}
In stmt I’m not sure what this means:
func (Between) expression() {}
Why
// and Limit, Offset, OrderBy, Prefix, Using, Values, Where
type Value struct { Value interface{} }
instead of
type Value interface{}
Generally I could see function types and closures being useful, I’m not
seeing those anywhere.
This kind of thing is a lot of work to get right and has the overhead of
copying anything assigned to an interface{}:
type Map map[interface{}]interface{}
Maybe there’s a better way?
Thanks for the MIT licensing.
Matt
On Tuesday, February 27, 2018 at 11:57:42 AM UTC-6, Thomas LE ROUX wrote:
>
> Hi,
>
> I'd like to announce a new open source project from Ulule: *Loukoum *
> (https://github.com/ulule/loukoum)
>
> *.*It's a simple query builder that will help you to generate complex
> queries without SQL injection.
> If you're not using an ORM, you should give it a try.
>
> At the moment, we only support *PostgreSQL. *
> But feel free to contribute and give your feedback :)
>
>
> Cheers,
> Thomas.
>
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
