Skip to content

Commit

Permalink
fix(sql): limit with joins
Browse files Browse the repository at this point in the history
json type for object literals.

fixes #160
  • Loading branch information
marcj committed Mar 28, 2022
1 parent a6f5734 commit e135620
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 79 deletions.
1 change: 1 addition & 0 deletions packages/mysql/src/mysql-platform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export class MySQLPlatform extends DefaultPlatform {
this.nativeTypeInformation.set('longblob', { needsIndexPrefix: true, defaultIndexSize: 767 });

this.addType(ReflectionKind.class, 'json');
this.addType(ReflectionKind.objectLiteral, 'json');
this.addType(ReflectionKind.array, 'json');
this.addType(ReflectionKind.union, 'json');

Expand Down
10 changes: 4 additions & 6 deletions packages/orm-integration/src/bookstore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,21 @@ import { isArray } from '@deepkit/core';
import { Group } from './bookstore/group';
import { DatabaseFactory } from './test';

class BookModeration {
locked: boolean = false;
interface BookModeration {
locked: boolean;

maxDate?: Date;

admin?: User;

moderators: User[] = [];
moderators: User[];
}

@entity.name('book')
class Book {
public id?: number & PrimaryKey & AutoIncrement;

moderation: BookModeration = new BookModeration;
moderation: BookModeration = { locked: false, moderators: [] };

constructor(
public author: User & Reference,
Expand Down Expand Up @@ -431,14 +431,12 @@ export const bookstoreTests = {
const book1DB = await database.query(Book).filter({ author: peter }).findOne();
expect(book1DB.title).toBe('Peters book');
expect(book1DB.moderation === book1.moderation).toBe(false);
expect(book1DB.moderation).toBeInstanceOf(BookModeration);
expect(book1DB.moderation.locked).toBe(true);
}

{
const book2DB = await database.query(Book).filter({ author: herbert }).findOne();
expect(book2DB.title).toBe('Herberts book');
expect(book2DB.moderation).toBeInstanceOf(BookModeration);
expect(book2DB.moderation.locked).toBe(false);
expect(book2DB.moderation.admin).toBeInstanceOf(User);
expect(book2DB.moderation.admin?.name).toBe('Admin');
Expand Down
32 changes: 31 additions & 1 deletion packages/orm-integration/src/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class User {

posts?: Post[] & BackReference;

groups?: Group[] & BackReference<{via: typeof UserGroup}>;
groups?: Group[] & BackReference<{via: UserGroup}>;

constructor(public username: string) {
}
Expand Down Expand Up @@ -117,4 +117,34 @@ export const usersTests = {
}
database.disconnect();
},

async limitWithJoins(databaseFactory: DatabaseFactory) {
const database = await databaseFactory(entities);

const user1 = new User('User1');
const user2 = new User('User2');

const post1 = new Post(user1, 'Post 1');
const post2 = new Post(user1, 'Post 2');
const post3 = new Post(user1, 'Post 3');

await database.persist(user1, user2, post1, post2, post3);

{
const user = await database.query(User).joinWith('posts').findOne();
expect(user).toBeInstanceOf(User);
expect(user.posts!.length).toBe(3);
expect(user.posts![0]).toBeInstanceOf(Post);
}

{
const users = await database.query(User).joinWith('posts').limit(1).find();
expect(users.length).toBe(1);
const user = users[0]!;

expect(user).toBeInstanceOf(User);
expect(user.posts!.length).toBe(3);
expect(user.posts![0]).toBeInstanceOf(Post);
}
}
};
1 change: 1 addition & 0 deletions packages/postgres/src/postgres-platform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export class PostgresPlatform extends DefaultPlatform {
this.addType(ReflectionKind.boolean, 'boolean');

this.addType(ReflectionKind.class, 'jsonb');
this.addType(ReflectionKind.objectLiteral, 'jsonb');
this.addType(ReflectionKind.array, 'jsonb');
this.addType(ReflectionKind.union, 'jsonb');

Expand Down
44 changes: 5 additions & 39 deletions packages/sql/src/sql-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,51 +259,17 @@ export class SQLQueryResolver<T extends OrmEntity> extends GenericQueryResolver<
if (formatterFrame) formatterFrame.end();

return results;
} catch (error) {
throw error;
throw new DatabaseError(`Could not query ${this.classSchema.getClassName()} due to SQL error ${error}.\nSQL: ${sql.sql}\nParams: ${JSON.stringify(sql.params)}`);
} catch (error: any) {
throw new DatabaseError(`Could not query ${this.classSchema.getClassName()} due to SQL error ${error}.\nSQL: ${sql.sql}\nParams: ${JSON.stringify(sql.params)}. Error: ${error}`);
} finally {
connection.release();
}
}

async findOneOrUndefined(model: SQLQueryModel<T>): Promise<T | undefined> {
const sqlBuilderFrame = this.session.stopwatch ? this.session.stopwatch.start('SQL Builder') : undefined;
const sqlBuilder = new SqlBuilder(this.platform);
const sql = sqlBuilder.select(this.classSchema, model);
if (sqlBuilderFrame) sqlBuilderFrame.end();

const connectionFrame = this.session.stopwatch ? this.session.stopwatch.start('Connection acquisition') : undefined;
const connection = await this.connectionPool.getConnection(this.session.logger, this.session.assignedTransaction, this.session.stopwatch);
if (connectionFrame) connectionFrame.end();
let row: any;

try {
row = await connection.execAndReturnSingle(sql.sql, sql.params);
if (!row) return;
} catch (error) {
throw new DatabaseError(`Could not query ${this.classSchema.getClassName()} due to SQL error ${error}.\nSQL: ${sql.sql}\nParams: ${JSON.stringify(sql.params)}`);
} finally {
connection.release();
}

if (model.isAggregate() || model.sqlSelect) {
//when aggregate the field types could be completely different, so don't normalize
return row;
}

const formatterFrame = this.session.stopwatch ? this.session.stopwatch.start('Formatter') : undefined;
try {
const formatter = this.createFormatter(model.withIdentityMap);
if (model.hasJoins()) {
const [converted] = sqlBuilder.convertRows(this.classSchema, model, [row]);
return formatter.hydrate(model, converted);
} else {
return formatter.hydrate(model, row);
}
} finally {
if (formatterFrame) formatterFrame.end();
}
//when joins are used, it's important to fetch all rows
const items = await this.find(model);
return items[0];
}

async has(model: SQLQueryModel<T>): Promise<boolean> {
Expand Down
69 changes: 36 additions & 33 deletions packages/sql/src/sql-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ export class SqlBuilder {
protected appendHavingSQL(sql: Sql, schema: ReflectionClass<any>, model: DatabaseQueryModel<any>, tableName: string) {
if (!model.having) return;

// tableName = tableName || this.platform.getTableIdentifier(schema);
const filter = getSqlFilter(schema, model.having, model.parameters, this.platform.serializer);
const builder = this.platform.createSqlFilterBuilder(schema, tableName);
builder.placeholderStrategy.offset = sql.params.length;
Expand Down Expand Up @@ -314,13 +313,45 @@ export class SqlBuilder {
}
}

public build<T>(schema: ReflectionClass<any>, model: SQLQueryModel<T>, head: string, withRange: boolean = true): Sql {
public build<T>(schema: ReflectionClass<any>, model: SQLQueryModel<T>, head: string): Sql {
const tableName = this.platform.getTableIdentifier(schema);
const sql = new Sql(`${head} FROM ${tableName}`, this.params);

const sql = new Sql(`${head} FROM`, this.params);

const withRange = model.limit !== undefined || model.skip !== undefined;
if (withRange && model.hasJoins()) {
//wrap FROM table => FROM (SELECT * FROM table LIMIT x OFFSET x)

sql.append(`(SELECT * FROM ${tableName}`);
this.platform.applyLimitAndOffset(sql, model.limit, model.skip);
sql.append(`) as ${tableName}`);
} else {
sql.append(tableName);
}

this.appendJoinSQL(sql, model, tableName);
this.appendWhereSQL(sql, schema, model);

if (withRange) {
if (model.groupBy.size) {
const groupBy: string[] = [];
for (const g of model.groupBy.values()) {
groupBy.push(`${tableName}.${this.platform.quoteIdentifier(g)}`);
}

sql.append('GROUP BY ' + groupBy.join(', '));
}

this.appendHavingSQL(sql, schema, model, tableName);

const order: string[] = [];
if (model.sort) {
for (const [name, sort] of Object.entries(model.sort)) {
order.push(`${tableName}.${this.platform.quoteIdentifier(name)} ${sort}`);
}
if (order.length) sql.append(' ORDER BY ' + (order.join(', ')));
}

if (withRange && !model.hasJoins()) {
this.platform.applyLimitAndOffset(sql, model.limit, model.skip);
}

Expand Down Expand Up @@ -351,34 +382,6 @@ export class SqlBuilder {
}
}

const tableName = this.platform.getTableIdentifier(schema);

const order: string[] = [];
if (model.sort) {
for (const [name, sort] of Object.entries(model.sort)) {
order.push(`${tableName}.${this.platform.quoteIdentifier(name)} ${sort}`);
}
}

const sql = this.build(schema, model, 'SELECT ' + (manualSelect || this.sqlSelect).join(', '), false);

if (model.groupBy.size) {
const groupBy: string[] = [];
for (const g of model.groupBy.values()) {
groupBy.push(`${tableName}.${this.platform.quoteIdentifier(g)}`);
}

sql.append('GROUP BY ' + groupBy.join(', '));
}

this.appendHavingSQL(sql, schema, model, tableName);

if (order.length) {
sql.append(' ORDER BY ' + (order.join(', ')));
}

this.platform.applyLimitAndOffset(sql, model.limit, model.skip);

return sql;
return this.build(schema, model, 'SELECT ' + (manualSelect || this.sqlSelect).join(', '));
}
}

0 comments on commit e135620

Please sign in to comment.