Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix for WrapMysqlModifySubqueryTransformation when using join builder #6

Closed
wants to merge 2 commits into from

Conversation

marcinlee
Copy link

@marcinlee marcinlee commented Jul 14, 2022

Fixes error where .onIn()/.andOnIn() causes:
TypeError: parentQuery.isUpdate is not a function

This happens when we provide .onIn() with an objection query.

BlogPost.query(db)
  .alias('post')
  .innerJoin('post.comment', (join) => {
    join.onIn(
      'comment.author_id',
      Author.query(db).where('active', true)
    );
  });

Since WrapMysqlModifySubqueryTransformation does not make sense for join query, we simply skip it for JoinBuilder.

Note: This wasn't an issue in objection v1 and this error does not happen if we provide .onIn() with knex query instead.

Copy link
Member

@falkenhawk falkenhawk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the PR!

@@ -2,6 +2,8 @@

const { QueryTransformation } = require('./QueryTransformation');
const { isMySql } = require('../../utils/knexUtils');
const { once } = require('../../utils/objectUtils');
const getJoinBuilder = once(() => require('../JoinBuilder').JoinBuilder);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't it a slight over-optimization? Wouldn't this suffice? :)

Suggested change
const getJoinBuilder = once(() => require('../JoinBuilder').JoinBuilder);
const { JoinBuilder } = require('../JoinBuilder');

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok it seems it is necessary - same logic is in

const getJoinBuilder = once(() => require('../JoinBuilder').JoinBuilder);

allow all pull_request to be tested, at any branch
@falkenhawk falkenhawk force-pushed the v3-mod-join-wrap-fix branch from ca5eef2 to a0b6c2a Compare August 10, 2022 07:21
Fixes and error where .andOnIn() causes:
"TypeError: parentQuery.isUpdate is not a function"
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants