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

Fix query builder crashing when using object in .where inside Brackets #4337

Closed
wants to merge 2 commits into from

Conversation

ephys
Copy link

@ephys ephys commented Jun 21, 2019

This is an attempt at fixing #3685 where the following piece of code

const generatedSql = entityManager
    .createQueryBuilder()
    .from(Product, 'Product')
    .where(new Brackets(qb => qb.where({ id: 1 })))

would crash because the QueryBuilder could not determine the alias to use for id

@pleerock
Copy link
Member

This small functionality got too complex in my opinion.

@tonivj5

This comment has been minimized.

@viczam

This comment has been minimized.

@imnotjames imnotjames self-requested a review October 14, 2020 09:32
* Creates a query builder that shares some context with another query-builder.
* Used for creating Bracket Query Builders.
*/
createBracketsQueryBuilder(): this {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be private

expressionMap.aliasNamePrefixingEnabled = this.expressionMap.aliasNamePrefixingEnabled;
expressionMap.nativeParameters = this.expressionMap.nativeParameters;

return new (this.constructor as any)(this.connection, this.queryRunner, expressionMap);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing that creating a new query builder by passing this query builder into it does not fix the issue? That would clone the expression map. eg use clone instead of this function

beforeEach(() => reloadTestingDatabases(connections));
after(() => connections && closeTestingConnections(connections));

it("should accept objects in .where method (github issue #3685)", () => Promise.all(connections.map(async connection => {
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to reference the issue in it but should explain what it should and should not do.


it("should accept objects in .where method (github issue #3685)", () => Promise.all(connections.map(async connection => {

await Promise.all([
Copy link
Contributor

Choose a reason for hiding this comment

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

const repo = connection.getRepository(User);

await Promise.all([repo.save({ first name: "Jean", last name:"Doe"})])

@AlexMesser
Copy link
Collaborator

Superseded by #5659. Anyway, thank you for contribution.

@AlexMesser AlexMesser closed this Apr 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants