j4mie / idiorm

A lightweight nearly-zero-configuration object-relational mapper and fluent query builder for PHP5.
http://j4mie.github.com/idiormandparis/
2.01k stars 369 forks source link

raw_join missing "JOIN" in Statement? #353

Closed riconeitzel closed 5 years ago

riconeitzel commented 5 years ago

I just stumbled upon this function and I noticed some SQL errors.

$rawJoin = 'keywords.name = search_console.name AND keywords.domain_id = domain.id';

$count = ORM::for_table('search_console')
                ->join('domain', ['search_console.domain', '=', 'domain.name'])
                ->raw_join('keywords', $rawJoin, NULL)
                ->where_null('search_console.keyword_id')
                ->count();

https://github.com/j4mie/idiorm/blob/ee3022fcf71232309112714ca4a7760105002f99/idiorm.php#L1066

In order to get this running I had to add "JOIN " in front of the referenced raw join string.

Is this a bug in idiorm or am I just holding it wrong?! 🤨

Thanks :-) Rico

treffynnon commented 5 years ago

When using raw join you must supply the JOIN keyword of your choice. This is by design as you might want a left, right, inner, etc join so it allows you to choose.

See the documentation for an example of it in action: https://idiorm.readthedocs.io/en/latest/querying.html#raw-join-clauses

I would also recommend changing your code to make use of the parameter binding rather than directly concatenating strings in your join clause. It's a safer way of supplying those variable values.

On Sat., 7 Sep. 2019, 00:34 Rico Neitzel, notifications@github.com wrote:

I just stumbled upon this function and I noticed some SQL errors.

$rawJoin = 'keywords.name = search_console.name AND keywords.domain_id = domain.id';

$count = ORM::for_table('search_console') ->join('domain', ['search_console.domain', '=', 'domain.name']) ->raw_join('keywords', $rawJoin, NULL) ->where_null('search_console.keyword_id') ->count();

https://github.com/j4mie/idiorm/blob/ee3022fcf71232309112714ca4a7760105002f99/idiorm.php#L1066

In order to get this running I had to add "JOIN " in front of the referenced raw join string.

Is this a bug in idiorm or am I just holding it wrong?! 🤨

Thanks :-) Rico

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/j4mie/idiorm/issues/353?email_source=notifications&email_token=AAAP5PY3NQERPFBFEC4WV63QIJS6TA5CNFSM4IUJWQZKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HJ2KKFQ, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAP5P7J6KIDVCVP6R6TDJLQIJS6TANCNFSM4IUJWQZA .

treffynnon commented 5 years ago

I am assuming this resolved your issue so I am closing this ticket. If you're still having problems then feel free to comment and I'll do my best to help further.

riconeitzel commented 5 years ago

Thanks for the heads up … I misunderstood the usage of the method. the signature implies, the first parameter is a TABLE name … instead it's the KEYWORD (join) AND the table name.

It would make more sense to have an additional parameter with the JOIN keyword.

thanks for your immediate help ;-)