LauJensen / clojureql

ClojureQL is superior SQL integration for Clojure
https://clojureql.sabrecms.com
Eclipse Public License 1.0
285 stars 39 forks source link

(join .. (aggregate .. (join .. ))) missing group-by columns #45

Closed bendlas closed 13 years ago

bendlas commented 13 years ago
(join (table :x)
       (aggregate (join (table :y) (table :z) :j)
                       [[:agg/x :as :x]]
                       [:id :z.id])
       :oj)

In which y.id and z.id are missing, which doesn't happen without the inner join

LauJensen commented 13 years ago

On MASTER is resolves to this stmt: SELECT x.*,y_subselect.x FROM x JOIN (SELECT y.id,z.id,agg(y.x) AS x FROM y JOIN z USING(j) GROUP BY y.id,z.id) AS y_subselect USING(oj)

What did you expect?

bendlas commented 13 years ago

SELECT x.*,y_subselect.x, y_subselect.id, y_subselect.z_id FROM x JOIN (SELECT y.id,z.id,agg(y.x) AS x FROM y JOIN z USING(j) GROUP BY y.id,z.id) AS y_subselect USING(oj)

Of course, z.id should have been named differently, but that changes nothing

Anyway, the group-by stmt only makes sense for columns being propagated further, otherwise one could leave the cols out in the first place

LauJensen commented 13 years ago

Bendlas,

Ive been looking at this and I see where the problem is, however I wonder about your expansion, what do you mean by "SELECT ....,y_subselect.z_id ...." ? z_id is defined nowhere in the query

bendlas commented 13 years ago

Yes, I wrote the expansion that way b/c otherwise y.id and z.id had the same name. Sorry for the added confusion.

Bendlas

LauJensen commented 13 years ago

No worries. But does that mean, that CQL has to detect the name collision and alias z.id both in the subselect and the fieldspec?

bendlas commented 13 years ago

Actually, this issue is independent from the name collision. It is about the columns mentioned in the inner GROUP BY clause, which should be propagated as if (project ..) ed. So

(join (table :x)
   (aggregate (join (table :y) (table :z) :j)
                   [[:agg/x :as :x]]
                   [:z.z_id])
   :oj)

should yield

SELECT x.*,y_subselect.x, y_subselect.z_id
   FROM x JOIN (SELECT z.z_id,agg(y.x) AS x FROM y
     JOIN z USING(j) GROUP BY z.z_id) AS y_subselect USING(oj)

currently the y_subselect.z_id is missing from the outer select.

Whether or not CQL should detect name clashes, would be more of a design decision, I guess.

LauJensen commented 13 years ago

Alright bendlas, I think this is fixed now. Could you please verify https://github.com/LauJensen/clojureql/commit/42fb5543d7c7df46575ef65645ef80e6a2725503

And reopen if there are any quirks. Thanks!