diff --git a/src/datomish/db.cljc b/src/datomish/db.cljc index 4e1b718f..ff4c7d91 100644 --- a/src/datomish/db.cljc +++ b/src/datomish/db.cljc @@ -643,10 +643,12 @@ "Execute the provided query on the provided DB. Returns a transduced channel of [result err] pairs. Closes the channel when fully consumed." - [db find args] - (let [parsed (query/parse find) + [db find options] + (let [{:keys [limit order-by args]} options + parsed (query/parse find) context (-> db query-context + (query/options-into-context limit order-by) (query/find-into-context parsed)) row-pair-transducer (projection/row-pair-transducer context) sql (query/context->sql-string context args) @@ -665,6 +667,8 @@ (defn sql-clause [context] - (let [inner + (let [inner-projection (projection/sql-projection-for-relation context) + inner (merge - {:select (projection/sql-projection-for-relation context) + {:select inner-projection ;; Always SELECT DISTINCT, because Datalog is set-based. ;; TODO: determine from schema analysis whether we can avoid ;; the need to do this. :modifiers [:distinct]} - (clauses/cc->partial-subquery (:cc context)))] - (if (:has-aggregates? context) - (merge - (when-not (empty? (:group-by-vars context)) - ;; We shouldn't need to account for types here, until we account for - ;; `:or` clauses that bind from different attributes. - {:group-by (map util/var->sql-var (:group-by-vars context))}) - {:select (projection/sql-projection-for-aggregation context :preag) - :modifiers [:distinct] - :from [:preag] - :with {:preag inner}}) - inner))) + (clauses/cc->partial-subquery (:cc context))) + + limit (:limit context) + order-by (:order-by-vars context)] + + (if (:has-aggregates? context) + (let [outer-projection (projection/sql-projection-for-aggregation context :preag)] + ;; Validate the projected vars against the ordering clauses. + (merge + (limit-and-order limit outer-projection order-by) + (when-not (empty? (:group-by-vars context)) + ;; We shouldn't need to account for types here, until we account for + ;; `:or` clauses that bind from different attributes. + {:group-by (map util/var->sql-var (:group-by-vars context))}) + {:select outer-projection + :modifiers [:distinct] + :from [:preag] + :with {:preag inner}})) + + ;; Otherwise, validate against the inner. + (merge + (limit-and-order limit inner-projection order-by) + inner)))) (defn context->sql-string [context args] (-> @@ -96,6 +128,14 @@ {} in)) +(defn options-into-context + [context limit order-by] + (when-not (or (and (integer? limit) + (pos? limit)) + (nil? limit)) + (raise "Invalid limit " limit {:limit limit})) + (assoc context :limit limit :order-by-vars order-by)) + (defn find-into-context "Take a parsed `find` expression and return a fully populated Context. You'll want this so you can get access to the diff --git a/src/datomish/query/context.cljc b/src/datomish/query/context.cljc index b82814ff..8a21d8cb 100644 --- a/src/datomish/query/context.cljc +++ b/src/datomish/query/context.cljc @@ -12,8 +12,10 @@ elements ; The :find list itself. has-aggregates? group-by-vars ; A list of variables from :find and :with, used to generate GROUP BY. + order-by-vars ; A list of projected variables and directions, e.g., [:date :asc], [:_max_timestamp :desc]. + limit ; The limit to apply to the final results of the query. Only makes sense with ORDER BY. cc ; The main conjoining clause. ]) (defn make-context [source] - (->Context source nil false nil nil)) + (->Context source nil false nil nil nil nil)) diff --git a/test/datomish/test/query.cljc b/test/datomish/test/query.cljc index 6482c421..a34a2f69 100644 --- a/test/datomish/test/query.cljc +++ b/test/datomish/test/query.cljc @@ -517,3 +517,53 @@ [?page :page/url _] [(get-else $ ?page :page/title "No title") ?title]] conn))))) + +(deftest-db test-limit-order conn + (let [attrs ( ?date ?then)] + [?e :foo/points ?v]] conn)] + (is + (thrown-with-msg? + ExceptionInfo #"Invalid limit \?x" + (query/options-into-context context '?x [[:date :asc]]))) + (is + (thrown-with-msg? + ExceptionInfo #"Ordering expressions must be :asc or :desc" + (query/context->sql-clause + (query/options-into-context context 10 [[:date :upsidedown]])))) + (is + (thrown-with-msg? + ExceptionInfo #"Ordering vars \#\{:nonexistent\} not a subset" + (query/context->sql-clause + (query/options-into-context context 10 [[:nonexistent :desc]])))) + (is + (= + {:limit 10} + (select-keys + (query/context->sql-clause + (query/options-into-context context 10 nil)) + [:order-by :limit] + ))) + (is + (= + {:order-by [[:date :asc]]} + (select-keys + (query/context->sql-clause + (query/options-into-context context nil [[:date :asc]])) + [:order-by :limit] + ))) + (is + (= + {:limit 10 + :order-by [[:date :asc]]} + (select-keys + (query/context->sql-clause + (query/options-into-context context 10 [[:date :asc]])) + [:order-by :limit] + ))))) diff --git a/test/datomish/tofinoish_test.cljc b/test/datomish/tofinoish_test.cljc index 0257904b..e4c2b7d4 100644 --- a/test/datomish/tofinoish_test.cljc +++ b/test/datomish/tofinoish_test.cljc @@ -176,8 +176,7 @@ [?id :session/startReason ?reason ?tx] [?tx :db/txInstant ?ts] (not-join [?id] - [?id :session/endReason _])] - {})) + [?id :session/endReason _])])) (defn > - rows - (sort-by (comp unchecked-negate third)) ;; TODO: these should be dates! - (take limit) + {:limit limit + :order-by [[:_max_time :desc]] + :args {:since since}}))] (map (fn [[uri title lastVisited]] - {:uri uri :title title :lastVisited lastVisited}))))))) + {:uri uri :title title :lastVisited lastVisited}) + rows))))) (defn