fixed issue with using limit with order by

gulpfile.ts

@@ -309,7 +309,9 @@ export class Gulpfile {
         chai.use(require("chai-as-promised"));
 
         return gulp.src(["./build/compiled/test/**/*.js"])
-            .pipe(mocha())
+            .pipe(mocha({
+                timeout: 5000
+            }))
             .pipe(istanbul.writeReports());
     }
 

src/persistence/SubjectOperationExecutor.ts

@@ -794,7 +794,7 @@ export class SubjectOperationExecutor {
 
             // if relation id still does not exist - we arise an error
             if (!relationId)
-                throw new Error(`Cannot insert object of ${relation.inverseRelation.entityTarget} type. Looks like its not persisted yet, or cascades are not set on the relation.`); // todo: better error message
+                throw new Error(`Cannot insert object of ${(newBindEntity.constructor as any).name} type. Looks like its not persisted yet, or cascades are not set on the relation.`); // todo: better error message
 
             const columns = relation.junctionEntityMetadata.columns.map(column => column.name);
             const values = relation.isOwning ? [ownId, relationId] : [relationId, ownId];

src/query-builder/QueryBuilder.ts

@@ -909,7 +909,10 @@ export class QueryBuilder<Entity> {
         const mainAliasName = this.fromTableName ? this.fromTableName : this.aliasMap.mainAlias.name;
         let scalarResults: any[];
         if (this.firstResult || this.maxResults) {
-            const [sql, parameters] = this.getSqlWithParameters(); // todo: fix for sql server. We cant skip order by here! // { skipOrderBy: true }
+            // we are skipping order by here because its not working in subqueries anyway
+            // to make order by working we need to apply it on a distinct query
+            const [sql, parameters] = this.getSqlWithParameters({ skipOrderBy: true });
+            const [selects, orderBys] = this.createOrderByCombinedWithSelectExpression("distinctAlias");
 
             const distinctAlias = this.connection.driver.escapeTableName("distinctAlias");
             const metadata = this.connection.getMetadata(this.fromEntity.alias.target);
@@ -922,12 +925,21 @@ export class QueryBuilder<Entity> {
                     return `${distinctAlias}.${propertyName}) as ids_${primaryColumn.name}`;
                 }
             }).join(", ");
+            if (selects.length > 0)
+                idsQuery += ", " + selects;
+
             idsQuery += ` FROM (${sql}) ${distinctAlias}`; // TODO: WHAT TO DO WITH PARAMETERS HERE? DO THEY WORK?
 
+            if (orderBys.length > 0) {
+                idsQuery += " ORDER BY " + orderBys;
+            } else {
+                idsQuery += ` ORDER BY "ids_${metadata.firstPrimaryColumn.name}"`; // this is required for mssql driver if firstResult is used. Other drivers don't care about it
+            }
+
             if (this.connection.driver instanceof SqlServerDriver) { // todo: temporary. need to refactor and make a proper abstraction
 
-                if (this.firstResult)
-                    idsQuery += ` ORDER BY "ids_${metadata.firstPrimaryColumn.name}" OFFSET ${this.firstResult} ROWS`;
+                if (this.firstResult) // todo: order by already is set above
+                    idsQuery += ` OFFSET ${this.firstResult} ROWS`;
                 if (this.maxResults)
                     idsQuery += " FETCH NEXT " + this.maxResults + " ROWS ONLY";
             } else {
@@ -1659,22 +1671,50 @@ export class QueryBuilder<Entity> {
             }).join(" ");
     }
 
-    protected createOrderByExpression() {
-
-        // if user specified a custom order then apply it
-        if (Object.keys(this.orderBys).length > 0)
-            return " ORDER BY " + Object.keys(this.orderBys).map(columnName => this.replacePropertyNames(columnName) + " " + this.orderBys[columnName]).join(", ");
+    protected createOrderByCombinedWithSelectExpression(parentAlias: string) {
 
         // if table has a default order then apply it
-        if (!this.fromTableName) {
+        let orderBys = this.orderBys;
+        if (!Object.keys(orderBys).length && !this.fromTableName) {
             const metadata = this.connection.getMetadata(this.aliasMap.mainAlias.target);
-            if (metadata.table.orderBy)
-                return " ORDER BY " + Object
-                        .keys(metadata.table.orderBy)
-                        .map(key => this.replacePropertyNames(key) + " " + metadata.table.orderBy![key])
-                        .join(", ");
+            orderBys = metadata.table.orderBy || {};
         }
 
+        const selectString = Object.keys(orderBys)
+            .map(columnName => {
+                const [alias, column] = columnName.split(".");
+                return this.connection.driver.escapeAliasName(parentAlias) + "." + this.connection.driver.escapeColumnName(alias + "_" + column);
+            })
+            .join(", ");
+
+        const orderByString = Object.keys(orderBys)
+            .map(columnName => {
+                const [alias, column] = columnName.split(".");
+                return this.connection.driver.escapeAliasName(parentAlias) + "." + this.connection.driver.escapeColumnName(alias + "_" + column) + " " + this.orderBys[columnName];
+            })
+            .join(", ");
+
+        return [selectString, orderByString];
+    }
+
+    protected createOrderByExpression() {
+
+        let orderBys = this.orderBys;
+
+        // if table has a default order then apply it
+        if (!Object.keys(orderBys).length && !this.fromTableName) {
+            const metadata = this.connection.getMetadata(this.aliasMap.mainAlias.target);
+            orderBys = metadata.table.orderBy || {};
+        }
+
+        // if user specified a custom order then apply it
+        if (Object.keys(orderBys).length > 0)
+            return " ORDER BY " + Object.keys(orderBys)
+                    .map(columnName => {
+                        return this.replacePropertyNames(columnName) + " " + this.orderBys[columnName];
+                    })
+                    .join(", ");
+
         return "";
     }
 

test/other-issues/limit-with-order-by/entity/Category.ts

@@ -0,0 +1,14 @@
+import {Table} from "../../../../src/decorator/tables/Table";
+import {PrimaryGeneratedColumn} from "../../../../src/decorator/columns/PrimaryGeneratedColumn";
+import {Column} from "../../../../src/decorator/columns/Column";
+
+@Table()
+export class Category {
+
+    @PrimaryGeneratedColumn()
+    id: number;
+
+    @Column()
+    name: string;
+
+}

test/other-issues/limit-with-order-by/entity/Post.ts

@@ -0,0 +1,23 @@
+import {Table} from "../../../../src/decorator/tables/Table";
+import {PrimaryGeneratedColumn} from "../../../../src/decorator/columns/PrimaryGeneratedColumn";
+import {Column} from "../../../../src/decorator/columns/Column";
+import {Category} from "./Category";
+import {ManyToMany} from "../../../../src/decorator/relations/ManyToMany";
+import {JoinTable} from "../../../../src/decorator/relations/JoinTable";
+
+@Table()
+export class Post {
+
+    @PrimaryGeneratedColumn()
+    id: number;
+
+    @Column()
+    title: string;
+
+    @ManyToMany(type => Category, {
+        cascadeInsert: true
+    })
+    @JoinTable()
+    categories: Category[];
+
+}

test/other-issues/limit-with-order-by/limit-with-order-by.ts

@@ -0,0 +1,63 @@
+import "reflect-metadata";
+import {setupTestingConnections, closeConnections, reloadDatabases} from "../../utils/test-utils";
+import {Connection} from "../../../src/connection/Connection";
+import {Post} from "./entity/Post";
+import {expect} from "chai";
+import {Category} from "./entity/Category";
+
+describe("other issues > using limit in conjunction with order by", () => {
+
+    let connections: Connection[];
+    before(async () => connections = await setupTestingConnections({
+        entities: [__dirname + "/entity/*{.js,.ts}"],
+        schemaCreate: true,
+        dropSchemaOnConnection: true,
+    }));
+    beforeEach(() => reloadDatabases(connections));
+    after(() => closeConnections(connections));
+
+    it("should persist successfully and return persisted entity", () => Promise.all(connections.map(async function(connection) {
+
+        // generate bulk array of posts with categories
+        const promises: Promise<any>[] = [];
+        for (let i = 1; i <= 100; i++) {
+
+            const post = new Post();
+            post.title = "Hello Post #" + i;
+            post.categories = [];
+
+            for (let i = 1; i <= 5; i++) {
+                const category = new Category();
+                category.name = "category #" + i;
+                post.categories.push(category);
+            }
+            promises.push(connection.entityManager.persist(post));
+        }
+
+        await Promise.all(promises);
+
+        // check if ordering by main object works correctly
+
+        const loadedPosts1 = await connection.entityManager
+            .createQueryBuilder(Post, "post")
+            .innerJoinAndSelect("post.categories", "categories")
+            .setMaxResults(10)
+            .orderBy("post.id", "DESC")
+            .getMany();
+
+        expect(loadedPosts1).not.to.be.empty;
+        loadedPosts1.length.should.be.equal(10);
+        loadedPosts1[0].id.should.be.equal(100);
+        loadedPosts1[1].id.should.be.equal(99);
+        loadedPosts1[2].id.should.be.equal(98);
+        loadedPosts1[3].id.should.be.equal(97);
+        loadedPosts1[4].id.should.be.equal(96);
+        loadedPosts1[5].id.should.be.equal(95);
+        loadedPosts1[6].id.should.be.equal(94);
+        loadedPosts1[7].id.should.be.equal(93);
+        loadedPosts1[8].id.should.be.equal(92);
+        loadedPosts1[9].id.should.be.equal(91);
+
+    })));
+
+});

tsconfig.json

@@ -3,7 +3,7 @@
     "compilerOptions": {
         "lib": ["es5", "es6", "dom"],
         "outDir": "build/compiled",
-        "target": "es5",
+        "target": "es6",
         "module": "commonjs",
         "moduleResolution": "node",
         "emitDecoratorMetadata": true,