Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow joins to nest conditional statements #1397

Merged
merged 5 commits into from
May 14, 2016
Merged

Allow joins to nest conditional statements #1397

merged 5 commits into from
May 14, 2016

Conversation

atiertant
Copy link
Contributor

fix #690

make this possible:

.leftOuterJoin('accounts', function () {
    this
      .on('users.id', 'account.user_id')
      .andOn(function () {
         this.on('users.state', '=', 'NULL');
         this.orOn('users.state', 'accounts.state');
      });
  });

@rhys-vdw
Copy link
Member

@atiertant Awesome work. Just requires test and documentation and we can get this in a release.

@atiertant
Copy link
Contributor Author

@rhys-vdw where could i add documentation ? didn't see this in CONTRIBUTING.md ...

@rhys-vdw
Copy link
Member

Yeah, sorry, we need a pull request template. PRs need tests and docs. The
documentation is just index.html (which is the website).

On Thu, May 12, 2016, 5:20 PM Alexandre Tiertant notifications@github.com
wrote:

@rhys-vdw https://github.com/rhys-vdw where could i add documentation ?
didn't see this in CONTRIBUTING.md ...


You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#1397 (comment)

@atiertant
Copy link
Contributor Author

@rhys-vdw hope all is correct

@rhys-vdw
Copy link
Member

@atiertant Thanks for following that up. Looks great. 👍

@rhys-vdw rhys-vdw merged commit a06b44a into knex:master May 14, 2016
@rhys-vdw
Copy link
Member

Just released this as knex 0.11.3

@villelahdenvuo
Copy link
Contributor

Buh, I was just trying achieve this and I read the docs and I was like why isn't this working even though the docs say it should, but once I found these issues I realized it's a brand new feature.

@helios1138
Copy link
Contributor

helios1138 commented Jul 30, 2016

@rhys-vdw knex 0.11.9 - not working

@atiertant
Copy link
Contributor Author

@helios1138 have you got a failling code?

@emaddoma
Copy link

My own attempt to use this failed.

q.innerJoin('account_meta_catalog', function() {
    this.on('account_meta_catalog.account_id','=','accounts.id');
    this.andOn(function(){
        this.on(knex.raw("(account_meta_catalog.catalog->'card'->>'has_medical_discount_plan')::boolean = true"));
        this.orOn(knex.raw("(account_meta_catalog.catalog->'card'->>'has_dental_discount_plan')::boolean = true"));
    });
});

Results in error "TypeError: listener must be a function"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow joins to nest conditional statements
5 participants