mojolicious / mojo-pg

Mojolicious :heart: PostgreSQL
https://metacpan.org/release/Mojo-Pg
Artistic License 2.0
101 stars 46 forks source link

Feedback on adding support for multi-column joins #55

Closed rsindlin closed 5 years ago

rsindlin commented 5 years ago

The current SQL::Abstract::Pg module only supports joining on a single key. I'd like to add support for multi-column joins, eg, to enable joining tables that use composite keys. This is analagous to the support already added for multi-column unique constraints in upserts (commit 4463a46f4c4a14df57c5697f5aa87a8d501e9ab7), but for select.

The goal is to generate a FROM clause that would look like

SELECT * FROM "foo" JOIN "bar" ON ("foo"."id" = "bar"."foo_id" AND "foo"."id2" = "bar"."foo_id2")

There are two reasonable options for extending the syntax. I've chosen one in PR #54 but would like the community to weigh in on it. I'll post each option as a separate comment for voting.

rsindlin commented 5 years ago

Option 1

Assume additional conditions are AND'd.

$pg->abstract->select(['foo', ['bar', 'foo.id' => 'bar.foo_id', 'foo.id2' => 'bar.foo_id2']]);

This is the approach currently used in #54. The rationale for this approach is that it seems very readable, and join conditions are separate from where conditions.

rsindlin commented 5 years ago

Option 2

Explicitly require an -and operator.

$pg->abstract->select(['foo', ['bar', -and => ['foo.id' => 'bar.foo_id', 'foo.id2' => 'bar.foo_id2']]]);

This approach would align with the SQL::Abstract convention that "things in arrays are OR'd". The rationale would be that join conditions are similar enough to where conditions to extend the convention to joins.

shadowcat-mst commented 5 years ago

I'm happy enough with the former, especially since I have plans to add a join syntax to SQL::Abstract itself that provides a more explicit API if users desire it, so I figure approach 2 is pretty much not worth it.

jhthorsen commented 5 years ago

I agree with @shadowcat-mst, that implicit "AND" (option one) makes sense. I also don't think it's limiting, since it could be extended later with a {-or => [...]} syntax if needed.

shadowcat-mst commented 5 years ago

I have a prototype of an explicit hash based syntax as a start in an sqla branch (not pushed, still experimenting too much) but the upshot to my mind is that you can focus on sugar for simple cases and I'll provide the manipulexity version when I stop being need sniped by other things