feat(postgres): minify aliases option (#11095)

Raphaël Ricard · Sushant
Commit 3fcb2d23 authored Aug 31, 2019 by Raphaël Ricard Committed by Sushant Aug 31, 2019
Showing with 204 additions and 30 deletions
.travis.yml
lib/dialects/abstract/query-generator.js
lib/dialects/postgres/query.js
lib/sequelize.js
test/config/config.js
test/integration/associations/belongs-to-many.test.js
test/integration/dialects/postgres/dao.test.js
test/integration/dialects/postgres/query.test.js
test/integration/sequelize.test.js
test/support.js
types/lib/sequelize.d.ts
--- a/.travis.yml
+++ b/.travis.yml
@@ -64,6 +64,12 @@ jobs:
    - stage: test
      node_js: '6'
      sudo: required
+      env: POSTGRES_VER=postgres-10 SEQ_PG_PORT=8991 SEQ_PG_MINIFY_ALIASES=1 DIALECT=postgres
+      script:
+        - npm run test-integration
+    - stage: test
+      node_js: '6'
+      sudo: required
      env: POSTGRES_VER=postgres-95 SEQ_PG_PORT=8990 DIALECT=postgres-native
    - stage: release
      node_js: '8'

--- a/lib/dialects/abstract/query-generator.js
+++ b/lib/dialects/abstract/query-generator.js
@@ -225,7 +225,7 @@ class QueryGenerator {
        options.exception = 'WHEN unique_violation THEN GET STACKED DIAGNOSTICS sequelize_caught_exception = PG_EXCEPTION_DETAIL;';
        valueQuery = `${`CREATE OR REPLACE FUNCTION pg_temp.testfunc(OUT response ${quotedTable}, OUT sequelize_caught_exception text) RETURNS RECORD AS ${delimiter}` +
-        ' BEGIN '}${valueQuery} INTO response; EXCEPTION ${options.exception} END ${delimiter
+          ' BEGIN '}${valueQuery} INTO response; EXCEPTION ${options.exception} END ${delimiter
        } LANGUAGE plpgsql; SELECT (testfunc.response).*, testfunc.sequelize_caught_exception FROM pg_temp.testfunc(); DROP FUNCTION IF EXISTS pg_temp.testfunc()`;
      } else {
        options.exception = 'WHEN unique_violation THEN NULL;';
@@ -411,8 +411,8 @@ class QueryGenerator {
    for (const key in attrValueHash) {
      if (modelAttributeMap && modelAttributeMap[key] &&
-          modelAttributeMap[key].autoIncrement === true &&
+        modelAttributeMap[key].autoIncrement === true &&
-          !this._dialect.supports.autoIncrement.update) {
+        !this._dialect.supports.autoIncrement.update) {
        // not allowed to update identity column
        continue;
      }
@@ -1146,6 +1146,12 @@ class QueryGenerator {
    let subJoinQueries = [];
    let query;
+    // Aliases can be passed through subqueries and we don't want to reset them
+    if (this.options.minifyAliases && !options.aliasesMapping) {
+      options.aliasesMapping = new Map();
+      options.aliasesByTable = {};
+    }
    // resolve table name options
    if (options.tableAs) {
      mainTable.as = this.quoteIdentifier(options.tableAs);
@@ -1274,12 +1280,14 @@ class QueryGenerator {
            offset: options.offset,
            limit: options.groupedLimit.limit,
            order: groupedLimitOrder,
+            aliasesMapping: options.aliasesMapping,
+            aliasesByTable: options.aliasesByTable,
            where,
            include,
            model
          },
          model
-        ).replace(/;$/, '')  }) AS sub`; // Every derived table must have its own alias
+        ).replace(/;$/, '')}) AS sub`; // Every derived table must have its own alias
        const placeHolder = this.whereItemQuery(Op.placeholder, true, { model });
        const splicePos = baseQuery.indexOf(placeHolder);
@@ -1329,7 +1337,8 @@ class QueryGenerator {
    // Add GROUP BY to sub or main query
    if (options.group) {
-      options.group = Array.isArray(options.group) ? options.group.map(t => this.quote(t, model)).join(', ') : this.quote(options.group, model);
+      options.group = Array.isArray(options.group) ? options.group.map(t => this.aliasGrouping(t, model, mainTable.as, options)).join(', ') :  this.aliasGrouping(options.group, model, mainTable.as, options);
      if (subQuery) {
        subQueryItems.push(` GROUP BY ${options.group}`);
      } else {
@@ -1399,6 +1408,12 @@ class QueryGenerator {
    return `${query};`;
  }
+  aliasGrouping(field, model, tableName, options) {
+    const src = Array.isArray(field) ? field[0] : field;
+    return this.quote(this._getAliasForField(tableName, src, options) || src, model);
+  }
  escapeAttributes(attributes, options, mainTableAs) {
    return attributes && attributes.map(attr => {
      let addTable = true;
@@ -1420,7 +1435,13 @@ class QueryGenerator {
        } else {
          deprecations.noRawAttributes();
        }
-        attr = [attr[0], this.quoteIdentifier(attr[1])].join(' AS ');
+        let alias = attr[1];
+        if (this.options.minifyAliases) {
+          alias = this._getMinifiedAlias(alias, mainTableAs, options);
+        }
+        attr = [attr[0], this.quoteIdentifier(alias)].join(' AS ');
      } else {
        attr = !attr.includes(Utils.TICK_CHAR) && !attr.includes('"')
          ? this.quoteAttribute(attr, options.model)
@@ -1502,7 +1523,13 @@ class QueryGenerator {
        } else {
          prefix = `${this.quoteIdentifier(includeAs.internalAs)}.${this.quoteIdentifier(attr)}`;
        }
-        return `${prefix} AS ${this.quoteIdentifier(`${includeAs.externalAs}.${attrAs}`, true)}`;
+        let alias = `${includeAs.externalAs}.${attrAs}`;
+        if (this.options.minifyAliases) {
+          alias = this._getMinifiedAlias(alias, includeAs.internalAs, topLevelInfo.options);
+        }
+        return `${prefix} AS ${this.quoteIdentifier(alias, true)}`;
      });
      if (include.subQuery && topLevelInfo.subQuery) {
        for (const attr of includeAttributes) {
@@ -1588,6 +1615,34 @@ class QueryGenerator {
    };
  }
+  _getMinifiedAlias(alias, tableName, options) {
+    // We do not want to re-alias in case of a subquery
+    if (options.aliasesByTable[`${tableName}${alias}`]) {
+      return options.aliasesByTable[`${tableName}${alias}`];
+    }
+    // Do not alias custom suquery_orders
+    if (alias.match(/subquery_order_[0-9]/)) {
+      return alias;
+    }
+    const minifiedAlias = `_${options.aliasesMapping.size}`;
+    options.aliasesMapping.set(minifiedAlias, alias);
+    options.aliasesByTable[`${tableName}${alias}`] = minifiedAlias;
+    return minifiedAlias;
+  }
+  _getAliasForField(tableName, field, options) {
+    if (this.options.minifyAliases) {
+      if (options.aliasesByTable[`${tableName}${field}`]) {
+        return options.aliasesByTable[`${tableName}${field}`];
+      }
+    }
+    return null;
+  }
  generateJoin(include, topLevelInfo) {
    const association = include.association;
    const parent = include.parent;
@@ -1627,9 +1682,15 @@ class QueryGenerator {
    if (topLevelInfo.options.groupedLimit && parentIsTop || topLevelInfo.subQuery && include.parent.subQuery && !include.subQuery) {
      if (parentIsTop) {
        // The main model attributes is not aliased to a prefix
-        joinOn = `${this.quoteTable(parent.as || parent.model.name)}.${this.quoteIdentifier(attrLeft)}`;
+        const tableName = this.quoteTable(parent.as || parent.model.name);
+        // Check for potential aliased JOIN condition
+        joinOn = this._getAliasForField(tableName, attrLeft, topLevelInfo.options) || `${tableName}.${this.quoteIdentifier(attrLeft)}`;
      } else {
-        joinOn = this.quoteIdentifier(`${asLeft.replace(/->/g, '.')}.${attrLeft}`);
+        const joinSource = `${asLeft.replace(/->/g, '.')}.${attrLeft}`;
+        // Check for potential aliased JOIN condition
+        joinOn = this._getAliasForField(asLeft, joinSource, topLevelInfo.options) || this.quoteIdentifier(joinSource);        
      }
    }
@@ -1672,11 +1733,17 @@ class QueryGenerator {
    const throughTable = through.model.getTableName();
    const throughAs = `${includeAs.internalAs}->${through.as}`;
    const externalThroughAs = `${includeAs.externalAs}.${through.as}`;
-    const throughAttributes = through.attributes.map(attr =>
+    const throughAttributes = through.attributes.map(attr => {
-      `${this.quoteIdentifier(throughAs)}.${this.quoteIdentifier(Array.isArray(attr) ? attr[0] : attr)
+      let alias = `${externalThroughAs}.${Array.isArray(attr) ? attr[1] : attr}`;
+      if (this.options.minifyAliases) {
+        alias = this._getMinifiedAlias(alias, throughAs, topLevelInfo.options);
+      }
+      return `${this.quoteIdentifier(throughAs)}.${this.quoteIdentifier(Array.isArray(attr) ? attr[0] : attr)
      } AS ${
-        this.quoteIdentifier(`${externalThroughAs}.${Array.isArray(attr) ? attr[1] : attr}`)}`
+        this.quoteIdentifier(alias)}`;
-    );
+    });
    const association = include.association;
    const parentIsTop = !include.parent.association && include.parent.model.name === topLevelInfo.options.model.name;
    const tableSource = parentTableName;
@@ -1717,9 +1784,15 @@ class QueryGenerator {
    // Used by both join and subquery where
    // If parent include was in a subquery need to join on the aliased attribute
    if (topLevelInfo.subQuery && !include.subQuery && include.parent.subQuery && !parentIsTop) {
-      sourceJoinOn = `${this.quoteIdentifier(`${tableSource}.${attrSource}`)} = `;
+      // If we are minifying aliases and our JOIN target has been minified, we need to use the alias instead of the original column name
+      const joinSource = this._getAliasForField(tableSource, `${tableSource}.${attrSource}`, topLevelInfo.options) || `${tableSource}.${attrSource}`;
+      sourceJoinOn = `${this.quoteIdentifier(joinSource)} = `;
    } else {
-      sourceJoinOn = `${this.quoteTable(tableSource)}.${this.quoteIdentifier(attrSource)} = `;
+      // If we are minifying aliases and our JOIN target has been minified, we need to use the alias instead of the original column name
+      const aliasedSource = this._getAliasForField(tableSource, attrSource, topLevelInfo.options) || attrSource;
+      sourceJoinOn = `${this.quoteTable(tableSource)}.${this.quoteIdentifier(aliasedSource)} = `;
    }
    sourceJoinOn += `${this.quoteIdentifier(throughAs)}.${this.quoteIdentifier(identSource)}`;
@@ -1892,6 +1965,7 @@ class QueryGenerator {
    if (Array.isArray(options.order)) {
      for (let order of options.order) {
        // wrap if not array
        if (!Array.isArray(order)) {
          order = [order];
@@ -1914,7 +1988,9 @@ class QueryGenerator {
          // see https://github.com/sequelize/sequelize/issues/8739
          const subQueryAttribute = options.attributes.find(a => Array.isArray(a) && a[0] === order[0] && a[1]);
          if (subQueryAttribute) {
-            order[0] = new Utils.Col(subQueryAttribute[1]);
+            const modelName = this.quoteIdentifier(model.name);
+            order[0] = new Utils.Col(this._getAliasForField(modelName, subQueryAttribute[1], options) || subQueryAttribute[1]);
          }
        }
@@ -2034,7 +2110,7 @@ class QueryGenerator {
          return this.whereItemsQuery(arg);
        }
        return this.escape(arg);
-      }).join(', ')  })`;
+      }).join(', ')})`;
    }
    if (smth instanceof Utils.Col) {
      if (Array.isArray(smth.col) && !factory) {

--- a/lib/dialects/postgres/query.js
+++ b/lib/dialects/postgres/query.js
@@ -74,7 +74,7 @@ class Query extends AbstractQuery {
      .then(queryResult => {
        complete();
-        const rows = Array.isArray(queryResult)
+        let rows = Array.isArray(queryResult)
          ? queryResult.reduce((allRows, r) => allRows.concat(r.rows || []), [])
          : queryResult.rows;
        const rowCount = Array.isArray(queryResult)
@@ -84,6 +84,17 @@ class Query extends AbstractQuery {
          )
          : queryResult.rowCount;
+        if (this.sequelize.options.minifyAliases && this.options.aliasesMapping) {
+          rows = rows
+            .map(row => _.toPairs(row)
+              .reduce((acc, [key, value]) => {
+                const mapping = this.options.aliasesMapping.get(key);
+                acc[mapping || key] = value;
+                return acc;
+              }, {})
+            );
+        }
        const isTableNameQuery = sql.startsWith('SELECT table_name FROM information_schema.tables');
        const isRelNameQuery = sql.startsWith('SELECT relname FROM pg_class WHERE oid IN');
@@ -153,7 +164,7 @@ class Query extends AbstractQuery {
              row.from = defParts[1];
              row.to = defParts[3];
              let i;
-              for (i = 5;i <= 8;i += 3) {
+              for (i = 5; i <= 8; i += 3) {
                if (/(UPDATE|DELETE)/.test(defParts[i])) {
                  row[`on_${defParts[i].toLowerCase()}`] = defParts[i + 1];
                }
@@ -173,7 +184,7 @@ class Query extends AbstractQuery {
              return m;
            }, {});
            result = rows.map(row => {
-              return _.mapKeys(row, (value, key)=> {
+              return _.mapKeys(row, (value, key) => {
                const targetAttr = attrsMap[key];
                if (typeof targetAttr === 'string' && targetAttr !== key) {
                  return targetAttr;
@@ -350,7 +361,7 @@ class Query extends AbstractQuery {
            parent: err
          });
        }
-        // falls through
+      // falls through
      default:
        return new sequelizeErrors.DatabaseError(err);
    }

--- a/lib/sequelize.js
+++ b/lib/sequelize.js
@@ -168,6 +168,7 @@ class Sequelize {
   * @param {boolean}  [options.typeValidation=false] Run built-in type validators on insert and update, and select with where clause, e.g. validate that arguments passed to integer fields are integer-like.
   * @param {Object}   [options.operatorsAliases] String based operator alias. Pass object to limit set of aliased operators.
   * @param {Object}   [options.hooks] An object of global hook functions that are called before and after certain lifecycle events. Global hooks will run after any model-specific hooks defined for the same event (See `Sequelize.Model.init()` for a list).  Additionally, `beforeConnect()`, `afterConnect()`, `beforeDisconnect()`, and `afterDisconnect()` hooks may be defined here.
+   * @param {boolean}  [options.minifyAliases=false] A flag that defines if aliases should be minified (mostly useful to avoid Postgres alias character limit of 64)
   */
  constructor(database, username, password, options) {
    let config;
@@ -254,7 +255,8 @@ class Sequelize {
      isolationLevel: null,
      databaseVersion: 0,
      typeValidation: false,
-      benchmark: false
+      benchmark: false,
+      minifyAliases: false
    }, options || {});
    if (!this.options.dialect) {

--- a/test/config/config.js
+++ b/test/config/config.js
@@ -76,6 +76,7 @@ module.exports = {
    pool: {
      max: env.SEQ_PG_POOL_MAX || env.SEQ_POOL_MAX || 5,
      idle: env.SEQ_PG_POOL_IDLE || env.SEQ_POOL_IDLE || 3000
-    }
+    },
+    minifyAliases: env.SEQ_PG_MINIFY_ALIASES
  }
 };
--- a/test/integration/associations/belongs-to-many.test.js
+++ b/test/integration/associations/belongs-to-many.test.js
@@ -1516,9 +1516,9 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), () => {
        this.comment = comment;
        this.tag = tag;
        return this.post.setTags([this.tag]);
-      }).then( () => {
+      }).then(() => {
        return this.comment.setTags([this.tag]);
-      }).then( () => {
+      }).then(() => {
        return Promise.all([
          this.post.getTags(),
          this.comment.getTags()
@@ -1559,7 +1559,7 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), () => {
        foreignKey: 'taggable_id'
      });
-      return this.sequelize.sync({ force: true }).then( () => {
+      return this.sequelize.sync({ force: true }).then(() => {
        return Promise.all([
          Post.create({ name: 'post1' }),
          Comment.create({ name: 'comment1' }),

--- a/test/integration/dialects/postgres/dao.test.js
+++ b/test/integration/dialects/postgres/dao.test.js
@@ -222,7 +222,7 @@ if (dialect.match(/^postgres/)) {
              }
            },
            logging(sql) {
-              expect(sql).to.equal('Executing (default): SELECT "id", "grappling_hook" AS "grapplingHook", "utilityBelt", "createdAt", "updatedAt" FROM "Equipment" AS "Equipment" WHERE "Equipment"."utilityBelt" = \'"grapplingHook"=>"true"\';');
+              expect(sql).to.contains(' WHERE "Equipment"."utilityBelt" = \'"grapplingHook"=>"true"\';');
            }
          });
        });
@@ -247,7 +247,7 @@ if (dialect.match(/^postgres/)) {
              }
            },
            logging(sql) {
-              expect(sql).to.equal('Executing (default): SELECT "id", "grappling_hook" AS "grapplingHook", "utilityBelt", "createdAt", "updatedAt" FROM "Equipment" AS "Equipment" WHERE CAST(("Equipment"."utilityBelt"#>>\'{grapplingHook}\') AS BOOLEAN) = true;');
+              expect(sql).to.contains(' WHERE CAST(("Equipment"."utilityBelt"#>>\'{grapplingHook}\') AS BOOLEAN) = true;');
            }
          });
        });

--- a/test/integration/dialects/postgres/query.test.js
+++ b/test/integration/dialects/postgres/query.test.js
+'use strict';
+const chai = require('chai'),
+  expect = chai.expect,
+  Support = require('../../support'),
+  dialect = Support.getTestDialect(),
+  DataTypes = require('../../../../lib/data-types');
+if (dialect.match(/^postgres/)) {
+  describe('[POSTGRES] Query', () => {
+    const taskAlias = 'AnActualVeryLongAliasThatShouldBreakthePostgresLimitOfSixtyFourCharacters';
+    const teamAlias = 'Toto';
+    const executeTest = (options, test) => {
+      const sequelize = Support.createSequelizeInstance(options);
+      const User = sequelize.define('User', { name: DataTypes.STRING, updatedAt: DataTypes.DATE }, { underscored: true });
+      const Team = sequelize.define('Team', { name: DataTypes.STRING });
+      const Task = sequelize.define('Task', { title: DataTypes.STRING });
+      User.belongsTo(Task, { as: taskAlias, foreignKey: 'task_id' });
+      User.belongsToMany(Team, { as: teamAlias, foreignKey: 'teamId', through: 'UserTeam' });
+      Team.belongsToMany(User, { foreignKey: 'userId', through: 'UserTeam' });
+      return sequelize.sync({ force: true }).then(() => {
+        return Team.create({ name: 'rocket' }).then(team => {
+          return Task.create({ title: 'SuperTask' }).then(task => {
+            return User.create({ name: 'test', task_id: task.id, updatedAt: new Date() }).then(user => {
+              return user[`add${teamAlias}`](team).then(() => {
+                return User.findOne({
+                  include: [
+                    {
+                      model: Task,
+                      as: taskAlias
+                    },
+                    {
+                      model: Team,
+                      as: teamAlias
+                    }
+                  ]
+                }).then(test);
+              });
+            });
+          });
+        });
+      });
+    };
+    it('should throw due to alias being truncated', function() {
+      const options = Object.assign({}, this.sequelize.options, { minifyAliases: false });
+      return executeTest(options, res => {
+        expect(res[taskAlias]).to.not.exist;
+      });
+    });
+    it('should be able to retrieve include due to alias minifying', function() {
+      const options = Object.assign({}, this.sequelize.options, { minifyAliases: true });
+      return executeTest(options, res => {
+        expect(res[taskAlias].title).to.be.equal('SuperTask');
+      });
+    });
+  });
+}
\ No newline at end of file
--- a/test/integration/sequelize.test.js
+++ b/test/integration/sequelize.test.js
@@ -1315,11 +1315,12 @@ describe(Support.getTestDialectTeaser('Sequelize'), () => {
        } else {
          it('correctly handles multiple transactions', function() {
            const TransactionTest = this.sequelizeWithTransaction.define('TransactionTest', { name: DataTypes.STRING }, { timestamps: false });
+            const aliasesMapping = new Map([['_0', 'cnt']]);
            const count = transaction => {
              const sql = this.sequelizeWithTransaction.getQueryInterface().QueryGenerator.selectQuery('TransactionTests', { attributes: [['count(*)', 'cnt']] });
-              return this.sequelizeWithTransaction.query(sql, { plain: true, transaction }).then(result => {
+              return this.sequelizeWithTransaction.query(sql, { plain: true, transaction, aliasesMapping  }).then(result => {
                return parseInt(result.cnt, 10);
              });
            };

--- a/test/support.js
+++ b/test/support.js
@@ -61,7 +61,8 @@ const Support = {
      dialect: options.dialect,
      port: options.port || process.env.SEQ_PORT || config.port,
      pool: config.pool,
-      dialectOptions: options.dialectOptions || config.dialectOptions || {}
+      dialectOptions: options.dialectOptions || config.dialectOptions || {},
+      minifyAliases: options.minifyAliases || config.minifyAliases
    });
    if (process.env.DIALECT === 'postgres-native') {

--- a/types/lib/sequelize.d.ts
+++ b/types/lib/sequelize.d.ts
@@ -347,6 +347,14 @@ export interface Options extends Logging {
   */
  hooks?: Partial<SequelizeHooks>;
+  /**
+   * Set to `true` to automatically minify aliases generated by sequelize.
+   * Mostly useful to circumvent the POSTGRES alias limit of 64 characters.
+   *
+   * @default false
+   */
+  minifyAliases?: boolean;
  retry?: RetryOptions;
 }