From 3004eaa7625f1d118e2f7cdc9bf3c5558ea21906 Mon Sep 17 00:00:00 2001
From: Matti Uusitalo <matti.uusitalo@solita.fi>
Date: Fri, 10 Mar 2023 11:07:01 +0200
Subject: [PATCH] Adds new distinct option to :options

when false or omitted, does SELECT ALL
when true, does SELECT DISTINCT
when a sequence, does SELECT DISTINCT ON (columns in that sequence)
---
 src/specql/impl/fetch.clj | 24 ++++++++++++++-
 test/database.sql         | 19 ++++++++++++
 test/specql/core_test.clj | 65 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/src/specql/impl/fetch.clj b/src/specql/impl/fetch.clj
index d91e83e..9fe4842 100644
--- a/src/specql/impl/fetch.clj
+++ b/src/specql/impl/fetch.clj
@@ -237,6 +237,26 @@
            path->array-type))
         results)))))
 
+(defn distinct-on [all-cols {distinct-opt :specql.core/distinct}]
+  (case distinct-opt
+    (nil false)
+    ""
+
+    true
+    " DISTINCT "
+
+    (do
+      (assert (seqable? distinct-opt))
+      (let [distinct-set (set distinct-opt)]
+        (str " DISTINCT ON ("
+             (str/join ", "
+                       (into []
+                             (comp
+                              (filter (fn [[_ [_ col _]]] (contains? distinct-set (first col))))
+                              (map (comp first second)))
+                             all-cols))
+             ") ")))))
+
 (defn- order-by [table-alias
                  columns
                  {order :specql.core/order-by
@@ -303,7 +323,9 @@
         (where/sql-where table-info-registry path->table where)
 
         all-cols (into cols has-many-join-cols)
-        sql (str "SELECT " (sql-columns-list all-cols)
+        sql (str "SELECT "
+                 (distinct-on all-cols options)
+                 (sql-columns-list all-cols)
                  " FROM " (sql-from table-info-registry table-alias)
                  (when-not (str/blank? where-clause)
                    (str " WHERE " where-clause))
diff --git a/test/database.sql b/test/database.sql
index 47cada2..7ffbb85 100644
--- a/test/database.sql
+++ b/test/database.sql
@@ -212,6 +212,25 @@ CREATE TABLE table_with_composite_key (
   name TEXT
 );
 
+CREATE TABLE parent_table (
+  id SERIAL PRIMARY KEY
+);
+
+CREATE TABLE with_possible_duplicates (
+  parent_id INTEGER REFERENCES parent_table (id),
+  "tag" text
+);
+
+INSERT INTO parent_table VALUES (1), (2), (3);
+
+INSERT INTO with_possible_duplicates VALUES
+(1, 'foo'),
+(1, 'bar'),
+(2, 'foo'),
+(3, 'bar')
+;
+
+
 -- Some tables for multiple has-many join tests
 
 -- CREATE TABLE customer (
diff --git a/test/specql/core_test.clj b/test/specql/core_test.clj
index 575c4f9..148368a 100644
--- a/test/specql/core_test.clj
+++ b/test/specql/core_test.clj
@@ -66,6 +66,9 @@
 
   ["recipient" :recipient/recipient]
   ["mailinglist" :mailinglist/mailinglist]
+
+  ["parent_table" :parent-table/parent-table]
+  ["with_possible_duplicates" :with-possible-duplicates/with-possible-duplicates]
   )
 
 (defmacro asserted [msg-regex & body]
@@ -103,6 +106,68 @@
                                   :employee/employees
                                   #{:employee/foo}
                                   {}))))
+(:with-possible-duplicates/with-possible-duplicates @specql.impl.registry/table-info-registry)
+(deftest distinct-option
+  (testing "Table contains expected data"
+    (is (= (list #:with-possible-duplicates{:parent_id 1 :tag "foo"}
+                 #:with-possible-duplicates{:parent_id 1 :tag "bar"}
+                 #:with-possible-duplicates{:parent_id 2 :tag "foo"}
+                 #:with-possible-duplicates{:parent_id 3 :tag "bar"})
+
+           (fetch db
+                  ;; table
+                  :with-possible-duplicates/with-possible-duplicates
+                  ;; columns
+                  #{:with-possible-duplicates/parent_id
+                    :with-possible-duplicates/tag}
+                  ;; where
+                  {}
+                  {::specql/order-by :with-possible-duplicates/parent_id}))))
+
+  (testing "Distinct false uses `SELECT ALL`"
+    (is (= (list #:with-possible-duplicates{:parent_id 1 :tag "foo"}
+                 #:with-possible-duplicates{:parent_id 1 :tag "bar"}
+                 #:with-possible-duplicates{:parent_id 2 :tag "foo"}
+                 #:with-possible-duplicates{:parent_id 3 :tag "bar"})
+
+           (fetch db
+                  ;; table
+                  :with-possible-duplicates/with-possible-duplicates
+                  ;; columns
+                  #{:with-possible-duplicates/parent_id
+                    :with-possible-duplicates/tag}
+                  ;; where
+                  {}
+                  {::specql/distinct false}))))
+
+
+  (testing "Distinct true removes duplicates"
+    (is (= (list #:with-possible-duplicates{:tag "foo"}
+                 #:with-possible-duplicates{:tag "bar"})
+
+           (fetch db
+                  ;; table
+                  :with-possible-duplicates/with-possible-duplicates
+                  ;; columns
+                  #{:with-possible-duplicates/tag}
+                  ;; where
+                  {}
+                  {::specql/distinct true}))))
+
+  (testing "Distinct with seq of columns removes duplicates across columns"
+    (is (= (list #:with-possible-duplicates{:parent_id 1 :tag "foo"}
+                 #:with-possible-duplicates{:parent_id 2 :tag "foo"}
+                 #:with-possible-duplicates{:parent_id 3 :tag "bar"})
+
+           (fetch db
+                  ;; table
+                  :with-possible-duplicates/with-possible-duplicates
+                  ;; columns
+                  #{:with-possible-duplicates/parent_id
+                    :with-possible-duplicates/tag}
+                  ;; where
+                  {}
+                  {::specql/distinct #{:with-possible-duplicates/parent_id}})))))
 
 (deftest query-with-invalid-parameter
   (let [x "foo"]