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

Querybuilder generating syntax errors #3501

Open
robbyemmert opened this issue Jan 24, 2019 · 23 comments
Open

Querybuilder generating syntax errors #3501

robbyemmert opened this issue Jan 24, 2019 · 23 comments

Comments

@robbyemmert
Copy link

Issue type:

[ x] question
[x ] bug report
[ ] feature request
[x ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[ ] mssql
[ ] mysql / mariadb
[ ] oracle
[ x] postgres
[ ] sqlite
[ ] sqljs
[ ] react-native
[ ] expo

TypeORM version:

[ ] latest
[ ] @next
[ ] ^0.2.7

QueryBuilder.getMany() and .getManyAndCount() are both wrapping my queries in a SELECT DISTINCT clause, which breaks my query when I use .orderBy. It's deceptive, because it's formatting the SQL different from .getSql(). The SQL from .getSql() works just fine when I run it manually.

I'm not sure why queryBuilder is wrapping my query in a SELECT DISTINCT clause... Is there a reason for this, or a way to disable it?

I'm not sure if this is a bug, question, or just something that needs better documentation.

Can .orderBy be used with queryBuilder?

My Code:

const query = this.serviceRepository.createQueryBuilder('service')
            .leftJoinAndSelect('service.type', 'service_type')
            // .where("LOWER(\"service\".\"countryName\") LIKE LOWER(:text)", {
            //     text: '%' + search.country + '%'
            // })
            // .orWhere("LOWER(\"service\".\"countryCode\") LIKE LOWER(:text)", {
            //     text: '%' + search.country + '%'
            // })
            .orderBy('\"service_countryName\"', 'ASC')
            .take(pageSettings.pageSize)
            .skip(pageSettings.pageSize * indexPage)

        const sql = await query.getSql()

        console.log('SQL OUT: ', sql)

        const services = await query.getMany()

The SQL as defined by .getSql() (Which works):

SELECT 
    "service"."id" AS "service_id", 
    "service"."created" AS "service_created", 
    "service"."updated" AS "service_updated", 
    "service"."notes" AS "service_notes", 
    "service"."active" AS "service_active", 
    "service"."associateFee" AS "service_associateFee", 
    "service"."governmentFee" AS "service_governmentFee", 
    "service"."miscMargin" AS "service_miscMargin", 
    "service"."chainGangMargin" AS "service_chainGangMargin", 
    "service"."typeId" AS "service_typeId", 
    "service"."countryId" AS "service_countryId", 
    "service"."countryCreated" AS "service_countryCreated", 
    "service"."countryUpdated" AS "service_countryUpdated", 
    "service"."countryName" AS "service_countryName", 
    "service"."countryCode" AS "service_countryCode", 
    "service_type"."id" AS "service_type_id", 
    "service_type"."created" AS "service_type_created", 
    "service_type"."updated" AS "service_type_updated", 
    "service_type"."name" AS "service_type_name", 
    "service_type"."description" AS "service_type_description" 
FROM 
    "service" "service" 
    LEFT JOIN "service_type" "service_type" ON "service_type"."id" = "service"."typeId" 
WHERE 
    LOWER("service"."countryName") LIKE LOWER($1) 
    OR LOWER("service"."countryCode") LIKE LOWER($2) 
ORDER BY 
    "service_countryName" ASC

The SQL that is actually hitting my database when using .getMany() or .getManyAndCount() (and failing)

SELECT 
    DISTINCT "distinctAlias"."service_id" as "ids_service_id", 
    "distinctAlias"."service_countryId" as "ids_service_countryId" 
FROM 
    (
        SELECT 
            "service"."id" AS "service_id", 
            "service"."created" AS "service_created", 
            "service"."updated" AS "service_updated", 
            "service"."notes" AS "service_notes", 
            "service"."active" AS "service_active", 
            "service"."associateFee" AS "service_associateFee", 
            "service"."governmentFee" AS "service_governmentFee", 
            "service"."miscMargin" AS "service_miscMargin", 
            "service"."chainGangMargin" AS "service_chainGangMargin", 
            "service"."typeId" AS "service_typeId", 
            "service"."countryId" AS "service_countryId", 
            "service"."countryCreated" AS "service_countryCreated", 
            "service"."countryUpdated" AS "service_countryUpdated", 
            "service"."countryName" AS "service_countryName", 
            "service"."countryCode" AS "service_countryCode", 
            "service_type"."id" AS "service_type_id", 
            "service_type"."created" AS "service_type_created", 
            "service_type"."updated" AS "service_type_updated", 
            "service_type"."name" AS "service_type_name", 
            "service_type"."description" AS "service_type_description" 
        FROM 
            "service" "service" 
            LEFT JOIN "service_type" "service_type" ON "service_type"."id" = "service"."typeId" 
        WHERE 
            LOWER("service"."countryName") LIKE LOWER($1) 
            OR LOWER("service"."countryCode") LIKE LOWER($2)
    ) "distinctAlias" 
ORDER BY 
    "service_countryName" ASC, 
    "service_id" ASC, 
    "service_countryId" ASC 
LIMIT 
    26 -- PARAMETERS: ["%%","%%"]

The associated logging error:

error: for SELECT DISTINCT, ORDER BY expressions must appear in select list

Thanks in advance!

@pleerock
Copy link
Member

pleerock commented Jan 25, 2019

when you use skip and take orm makes complex stuff to make sure you get your expected result. Complex stuff breaks when you use complex order bys.

But your order by doesn't look complex. In your case what is service_countryName? Why its not service.countryName ? Try to change it.

@robbyemmert
Copy link
Author

When I try it that way, I get the following error:

Error: ""service" alias was not found. Maybe you forgot to join it?

Any ideas?

@robbyemmert
Copy link
Author

I think it's because the ORDER BY clause is getting added to the wrapped query. If it's added to the inner query, I think it should work.

@d30jeff
Copy link

d30jeff commented Apr 26, 2019

Is there a workaround for this?

@garlou
Copy link

garlou commented Jun 14, 2019

It should work fine if you use offset and limit instead of skip and take cause it wont produce a subquery.

@Mando75
Copy link

Mando75 commented Jul 1, 2019

I think @robbyemmert is right, the order by is getting added to the outside query when it should only be on the inside (from what I can tell). I can confirm that using offset and limit does work as a workaround. I was having the same issue using skip and take when implementing a pretty simple pagination based on created date. Switching to offset and limit fixed it. That being said, the docs say specifically that when implementing pagination you should use skip and take so I would be interested in a real fix to those two methods. Just a note, I also tried specifying the order on the entity itself

@Entity("myEntity", {
   orderBy: {
      createdDate: "ASC"
   }
}

and that broke it too even when no orderBy call was made to the query builder.

@anodynos
Copy link

anodynos commented Aug 5, 2019

Came across this nasty bug - left comment at this PR instead.

@Insidexa
Copy link

We also faced such a problem (

@wojciechbator
Copy link

yeah, same here. Suppose such case:
because there's no support for selecting computed columns using addSelect('COUNT(prop) as result') I got the count using subquery in add select. Sorting works great until I use skip and take methods for pagination (this is paginated table). Then, the query is wrapped into distinctAlias and I can't select my computed field anymore, it's hidden in wrapped select and not mapped in outer select (which would allow me to sort over the computed field). Do you have idea of how to sort on computed column together with pagination using this lib?

@tuturis
Copy link

tuturis commented Apr 15, 2020

any solution for this? I'm having the same issue. Is there a way to resolve this?

@Evitcani
Copy link

I'm still having this exact issue. And finding wonky behavior when using offset and limit.

@Mando75
Copy link

Mando75 commented Sep 22, 2020

Sorry, I forgot to update this issue, but I've been able to work around this by adding an alias to my orderBy:

const reviews = await connection
      .getRepository(Review)
      .createQueryBuilder("review")
      .leftJoinAndSelect("review.book", "book")
      .orderBy({ "review.rating": "DESC" })
      .skip(10)
      .take(20)
      .getMany();
                                

@iurevych
Copy link

Looking at the last comment, while the alias format is working it is still failing for me when I have to wrap the alias into CHAR_LENGTH construction:

const reviews = await connection
      .getRepository(Review)
      .createQueryBuilder("review")
      .leftJoinAndSelect("review.book", "book")
      .orderBy('CHAR_LENGTH(review.rating)')
      .skip(10)
      .take(20)
      .getMany();

@ondratra
Copy link

Replacing skip/take for limit/offset works for me as well. Does limit and offset have any noticeable downsides? It is unclear to me if I can break something by this change, and there seems to be no mention of the difference in the docs.

@imnotjames
Copy link
Contributor

limit/offset will fail to work as expected when you're doing joins.

@AllanPinheiroDeLima
Copy link

It should work fine if you use offset and limit instead of skip and take cause it wont produce a subquery.

I had a similar problem... mine was having an ORDER BY FIELD(field, ?, ?) statement and when I added the leftJoin it broke.
But analizing the query, I could realize that it wasn't exactly my fault, but it was the ORM doing complex transformations because of take and skip

I have added your suggestion and my query now works

Thanks!

@BitPatty
Copy link
Contributor

BitPatty commented Jan 3, 2022

Remove the methods skip, take from query builder and implement them using JS .slice works for me. like it:

const entities = queryBuilder.getMany()
const paginatedEntities = entities.slice(skip, skip + take)

If you need handle a custom column (not mapped on typeorm entity) use .getRawAndEntities() instead getMany()

Note: I think to use typeorm skip / take and JS .slice have the same time complexity, O(n)

Your time blocking the thread, bandwidth utilization, memory usage and electricity bill will also have a complexity of O(n) 🔥

Seriously though, unless you only ever have a couple of entries in your database you should never do this. ​Let your database crunch the data - that's what it's for after all.

@kmordan24
Copy link

This worked for me: #747 (comment)

For some reason the combination of JOIN + ORDERBY + TAKE/SKIP
1.) fails when you use the column name instead of entity property name. For example, firstName is property name, first_name is column name.
2.) fails when you use what you would think would be the alias that typeorm creates for you (for example: table_name_column_name).

Try changing orderBy to "table_name.attributeName" - hopefully it works!

@jcuna
Copy link

jcuna commented Feb 1, 2022

I'm experiencing these issues with most recent versions of MySQL where an ORDER BY column has to be included in the distinct select which TYPEORM wraps my query in. I get the following error.

"Expression #1 of ORDER BY clause is not in SELECT list, references column 'distinctAlias.User_email' which is not in SELECT list; this is incompatible with DISTINCT" Would be great to just add a feature to allow to customize computed queries.

@jcuna
Copy link

jcuna commented Feb 1, 2022

This worked for me: #747 (comment)

For some reason the combination of JOIN + ORDERBY + TAKE/SKIP 1.) fails when you use the column name instead of entity property name. For example, firstName is property name, first_name is column name. 2.) fails when you use what you would think would be the alias that typeorm creates for you (for example: table_name_column_name).

Try changing orderBy to "table_name.attributeName" - hopefully it works!

Actually this helped. Switching my order by column from Alias_column to Alias.column helped as it avoids generating a computed distinct wrapper query.

Thanks

@FoxxMD
Copy link

FoxxMD commented Apr 19, 2022

Also experiencing this issue. None of the provided workarounds that don't use limit/offset work for me though -- i have joins.

@Niklas-Mezynski
Copy link

I could find a workaround for my case. I am using complex ORDER BY, SKIP and TAKE.
I am putting my order by attribute to the select statement with .addSelect
Broken code:

this.createQueryBuilder('a')
            .leftJoin(`tableB`, 'b, 'b.id=a.id')
            .skip(offset)
            .take(limit);
            .addOrderBy(
                  `(SELECT count(*) as count FROM tableC c WHERE c.some_id = a.id)`,
                  'DESC',
            )
            .getManyAndCount();

And I could fix it using

this.createQueryBuilder('a')
            .leftJoin(`tableB`, 'b, 'b.id=a.id')
            .skip(offset)
            .take(limit);
            .addSelect(
                  `SELECT count(*) as count FROM tableC c WHERE c.some_id = a.id)`,
                  'some_count',
            );
            .addOrderBy(`some_count`, 'DESC');
            .getManyAndCount();

Hope this helps

@noscripter
Copy link

noscripter commented Feb 29, 2024

Just wondering why it still exists,🤣(I can reproduce it in mysql server v8)

Thanks @garlou, you trick did work!

#3501 (comment)

Quickfix summary:

     const apps = await queryBuilder
-      .skip(pageNum - 1)
-      .take(pageSize)
+      .offset(pageNum - 1)
+      .limit(pageSize)
       .orderBy({ application_created_at: 'DESC' })
       .getMany();

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

No branches or pull requests