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: use aliases if subQuery is not used (v7) #14882

Merged
merged 16 commits into from Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 16 additions & 10 deletions src/dialects/abstract/query-generator.js
Expand Up @@ -1739,6 +1739,12 @@ export class AbstractQueryGenerator {
return null;
}

_getAliasForFieldFromQueryOptions(field, options) {
return (options.attributes || []).find(
WikiRik marked this conversation as resolved.
Show resolved Hide resolved
attr => Array.isArray(attr) && attr[1] && (attr[0] === field || attr[1] === field),
);
}

generateJoin(include, topLevelInfo, options) {
const association = include.association;
const parent = include.parent;
Expand Down Expand Up @@ -2146,21 +2152,21 @@ export class AbstractQueryGenerator {
// TODO - refactor this.quote() to not change the first argument
const field = model.rawAttributes[order[0]]?.field || order[0];
const subQueryAlias = this._getAliasForField(this.quoteIdentifier(model.name), field, options);

subQueryOrder.push(this.quote(subQueryAlias === null ? order : subQueryAlias, model, '->', options));
}

if (subQuery) {
// Handle case where sub-query renames attribute we want to order by,
// see https://github.com/sequelize/sequelize/issues/8739
// need to check if either of the attribute options match the order
const subQueryAttribute = options.attributes.find(a => Array.isArray(a)
&& a[1]
&& (a[0] === order[0] || (a[1] === order[0])));

if (subQueryAttribute) {
// Handle case where renamed attributes are used to order by,
// see https://github.com/sequelize/sequelize/issues/8739
// need to check if either of the attribute options match the order
if (options.attributes && model) {
const aliasedAttribute = this._getAliasForFieldFromQueryOptions(order[0], options);

if (aliasedAttribute) {
const modelName = this.quoteIdentifier(model.name);
const alias = this._getAliasForField(modelName, aliasedAttribute[1], options);

order[0] = new Utils.Col(this._getAliasForField(modelName, subQueryAttribute[1], options) || subQueryAttribute[1]);
order[0] = new Utils.Col(alias || aliasedAttribute[1]);
}
}

Expand Down
35 changes: 29 additions & 6 deletions src/dialects/mssql/query-generator.js
Expand Up @@ -996,15 +996,38 @@ export class MsSqlQueryGenerator extends AbstractQueryGenerator {
}

if (options.limit || options.offset) {
// TODO: document why this is adding the primary key of the model in ORDER BY
// if options.include is set
// TODO: document why this is adding the primary key of the model in ORDER BY if options.include is set
if (!options.order || options.order.length === 0 || options.include && orders.subQueryOrder.length === 0) {
const tablePkFragment = `${this.quoteTable(options.tableAs || model.name)}.${this.quoteIdentifier(model.primaryKeyField)}`;
if (!options.order || options.order.length === 0) {
let primaryKey = model.primaryKeyField;
const tablePkFragment = `${this.quoteTable(options.tableAs || model.name)}.${this.quoteIdentifier(primaryKey)}`;
const aliasedAttribute = this._getAliasForFieldFromQueryOptions(primaryKey, options);

if (aliasedAttribute) {
const modelName = this.quoteIdentifier(options.tableAs || model.name);
const alias = this._getAliasForField(modelName, aliasedAttribute[1], options);

primaryKey = new Utils.Col(alias || aliasedAttribute[1]);
sdepold marked this conversation as resolved.
Show resolved Hide resolved
}

if (!orders.mainQueryOrder || orders.mainQueryOrder.length === 0) {
fragment += ` ORDER BY ${tablePkFragment}`;
} else {
const orderFieldNames = _.map(options.order, order => order[0]);
const primaryKeyFieldAlreadyPresent = _.includes(orderFieldNames, model.primaryKeyField);
const orderFieldNames = (options.order || []).map(order => {
WikiRik marked this conversation as resolved.
Show resolved Hide resolved
const value = Array.isArray(order) ? order[0] : order;

if (value instanceof Utils.Col) {
return value.col;
}

if (value instanceof Utils.Literal) {
return value.val;
}

return value;
});
const primaryKeyFieldAlreadyPresent = orderFieldNames.includes(
(primaryKey.col || primaryKey),
);

if (!primaryKeyFieldAlreadyPresent) {
fragment += options.order && !isSubQuery ? ', ' : ' ORDER BY ';
Expand Down
34 changes: 34 additions & 0 deletions test/integration/dialects/postgres/query.test.js
Expand Up @@ -174,5 +174,39 @@ if (dialect.startsWith('postgres')) {
})).map(f => f.name);
expect(thisShouldAlsoWork[0]).to.equal('record1');
});

it('returns the minified aliased attributes', async () => {
const sequelizeMinifyAliases = Support.createSequelizeInstance({
logQueryParameters: true,
benchmark: true,
minifyAliases: true,
define: {
timestamps: false,
},
});

const Foo = sequelizeMinifyAliases.define(
'Foo',
{
name: {
field: 'my_name',
type: DataTypes.TEXT,
},
},
{ timestamps: false },
);

await sequelizeMinifyAliases.sync({ force: true });

await Foo.findAll({
subQuery: false,
attributes: {
include: [
[sequelizeMinifyAliases.literal('"Foo".my_name'), 'order_0'],
],
},
order: [['order_0', 'DESC']],
});
});
});
}
8 changes: 4 additions & 4 deletions test/unit/sql/select.test.js
Expand Up @@ -320,8 +320,8 @@ describe(Support.getTestDialectTeaser('SQL'), () => {
}, {
default: `SELECT [user].*, [POSTS].[id] AS [POSTS.id], [POSTS].[title] AS [POSTS.title] FROM (${
[
`SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 1 ORDER BY [user].[last_name] ASC${sql.addLimitAndOffset({ limit: 3, order: [['last_name', 'ASC']] })}) AS sub`,
`SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 5 ORDER BY [user].[last_name] ASC${sql.addLimitAndOffset({ limit: 3, order: [['last_name', 'ASC']] })}) AS sub`,
`SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 1 ORDER BY [lastName] ASC${sql.addLimitAndOffset({ limit: 3, order: [['last_name', 'ASC']] })}) AS sub`,
`SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 5 ORDER BY [lastName] ASC${sql.addLimitAndOffset({ limit: 3, order: [['last_name', 'ASC']] })}) AS sub`,
].join(current.dialect.supports['UNION ALL'] ? ' UNION ALL ' : ' UNION ')
}) AS [user] LEFT OUTER JOIN [post] AS [POSTS] ON [user].[id] = [POSTS].[user_id];`,
});
Expand Down Expand Up @@ -414,8 +414,8 @@ describe(Support.getTestDialectTeaser('SQL'), () => {
}, {
default: `SELECT [user].*, [POSTS].[id] AS [POSTS.id], [POSTS].[title] AS [POSTS.title], [POSTS->COMMENTS].[id] AS [POSTS.COMMENTS.id], [POSTS->COMMENTS].[title] AS [POSTS.COMMENTS.title] FROM (${
[
`SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 1 ORDER BY [user].[last_name] ASC${sql.addLimitAndOffset({ limit: 3, order: ['last_name', 'ASC'] })}) AS sub`,
`SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 5 ORDER BY [user].[last_name] ASC${sql.addLimitAndOffset({ limit: 3, order: ['last_name', 'ASC'] })}) AS sub`,
`SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 1 ORDER BY [lastName] ASC${sql.addLimitAndOffset({ limit: 3, order: ['last_name', 'ASC'] })}) AS sub`,
`SELECT * FROM (SELECT [id_user] AS [id], [email], [first_name] AS [firstName], [last_name] AS [lastName] FROM [users] AS [user] WHERE [user].[companyId] = 5 ORDER BY [lastName] ASC${sql.addLimitAndOffset({ limit: 3, order: ['last_name', 'ASC'] })}) AS sub`,
].join(current.dialect.supports['UNION ALL'] ? ' UNION ALL ' : ' UNION ')
}) AS [user] LEFT OUTER JOIN [post] AS [POSTS] ON [user].[id] = [POSTS].[user_id] LEFT OUTER JOIN [comment] AS [POSTS->COMMENTS] ON [POSTS].[id] = [POSTS->COMMENTS].[post_id];`,
});
Expand Down