walkable-server / walkable

A Clojure(script) SQL library for building APIs: Datomic® (GraphQL-ish) pull syntax, data driven configuration, dynamic filtering with relations in mind
https://walkable.gitlab.io/
Eclipse Public License 2.0
445 stars 15 forks source link

Pre-Condtion in `sqb/compile-schema` suggests only `columns` and `idents` is needed, but fails later for several other keys #54

Closed christoph-frick closed 6 years ago

christoph-frick commented 6 years ago

See https://github.com/walkable-server/walkable/blob/9621f1ca5fba0dbed06872c80ede5a7b646af3c7/src/walkable/sql_query_builder.cljc#L422

Due to the use of flatten-multi-keys later, also the following keys are basically mandatory for a schema:

While the first two most likely always exist anyways in any non-trivial example, extra-conditions might actually not be needed and the assert thrown later in flatten-multi-keys is way more confusing than just making them req-un for now?

myguidingstar commented 6 years ago

Great discovery! I think the above three should not be mandatory. flatten-multi-keys should only be applied after checking for non nil. Eg:

(flatten-multi-keys extra-conditions)

should become

(when extra-conditions
  (flatten-multi-keys extra-conditions))

are you willing to make another pr? :)

christoph-frick commented 6 years ago

What about putting them in the :or as empty maps?

https://github.com/walkable-server/walkable/blob/9621f1ca5fba0dbed06872c80ede5a7b646af3c7/src/walkable/sql_query_builder.cljc#L419

myguidingstar commented 6 years ago

that should work, too. Maybe that would save us from some null point exception headaches that I can't contemplate atm