search

fulcrologic / fulcro-rad-sql

SQL Plugin for Fulcro RAD
MIT License
4 stars 12 forks source link

Support cardinality :many in MS SQL: replace array_agg with Clojure fn #17

Closed danskarda closed 1 year ago

danskarda commented 1 year ago

to-many-join-column-query uses GROUP BY and array_agg aggregate function. Unfortunately not all SQL databases implement it, SQL Server is one of them (#15).

I propose to move this functionality out of SQL to Clojure function. Return all relations and group them in Clojure code. This makes it less dependent on specific SQL implementations. One drawback could be performance (it transfers more data) but I estimate the impact will be minimal. I guess performance is not main fulcro-rad-sql concern and its performance can be tuned elsewhere (roundtrips, formattign sqls with ids).

I considered also solution with string_agg, which strangely is implemented in MS SQL. But I decided against as primary keys can have different types and this could lead to mess.

awkay commented 1 year ago

Does this test out against the other databases? Seems like a perfectly good idea to me, and I'm willing to merge it. I don't use this adapter, so it won't get much testing from me.

awkay commented 1 year ago

If you'll squash this and confirm it is tested, I'll merge and release it.