fix: unify returning queries

sequelize · Mar 19, 2024 · d1d77f3 · d1d77f3
1 parent 78735c0
commit d1d77f3
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 184 deletions.
diff --git a/packages/core/src/dialects/db2/query-interface.js b/packages/core/src/dialects/db2/query-interface.js
@@ -78,9 +78,7 @@ export class Db2QueryInterface extends Db2QueryInterfaceTypeScript {
 
     delete options.replacements;
 
-    const result = await this.sequelize.queryRaw(sql, options);
-
-    return [result, undefined];
+    return this.sequelize.queryRaw(sql, options);
   }
 
   // TODO: drop "schema" options from the option bag, it must be passed through tableName instead.

diff --git a/packages/core/src/dialects/db2/query.js b/packages/core/src/dialects/db2/query.js
@@ -189,20 +189,49 @@ export class Db2Query extends AbstractQuery {
    * @private
    */
   formatResults(data, rowCount, metadata) {
-    let result = this.instance;
-    if (this.isInsertQuery(data, metadata)) {
-      this.handleInsertQuery(data, metadata);
+    if (this.isInsertQuery() || this.isUpdateQuery() || this.isUpsertQuery()) {
+      if (this.instance && this.instance.dataValues) {
+        // If we are creating an instance, and we get no rows, the create failed but did not throw.
+        // This probably means a conflict happened and was ignored, to avoid breaking a transaction.
+        if (this.isInsertQuery() && !this.isUpsertQuery() && data.length === 0) {
+          throw new sequelizeErrors.EmptyResultError();
+        }
 
-      if (!this.instance) {
-        if (this.options.plain) {
-          const record = data[0];
-          result = record[Object.keys(record)[0]];
-        } else {
-          result = data;
+        // Due to Db2 returning values with every insert or update,
+        // we only want to map the returned values to the instance if the user wants it.
+        // TODO: This is a hack, and should be fixed in the future.
+        if (this.options.returning && Array.isArray(data) && data[0]) {
+          for (const attributeOrColumnName of Object.keys(data[0])) {
+            const modelDefinition = this.model.modelDefinition;
+            const attribute = modelDefinition.columns.get(attributeOrColumnName);
+            const updatedValue = this._parseDatabaseValue(
+              data[0][attributeOrColumnName],
+              attribute?.type,
+            );
+
+            this.instance.set(attribute?.attributeName ?? attributeOrColumnName, updatedValue, {
+              raw: true,
+              comesFromDatabase: true,
+            });
+          }
         }
       }
+
+      if (this.isUpsertQuery()) {
+        return [this.instance, null];
+      }
+
+      return [
+        this.instance || (data && ((this.options.plain && data[0]) || data)) || undefined,
+        this.options.returning ? data.length : rowCount,
+      ];
     }
 
+    if (this.isBulkUpdateQuery()) {
+      return this.options.returning ? this.handleSelectQuery(data) : rowCount;
+    }
+
+    let result = this.instance;
     if (this.isDescribeQuery()) {
       result = {};
       for (const _result of data) {
@@ -223,16 +252,10 @@ export class Db2Query extends AbstractQuery {
       result = this.handleShowIndexesQuery(data);
     } else if (this.isSelectQuery()) {
       result = this.handleSelectQuery(data);
-    } else if (this.isUpsertQuery()) {
-      result = data;
     } else if (this.isCallQuery()) {
       result = data;
-    } else if (this.isBulkUpdateQuery()) {
-      result = data.length;
     } else if (this.isDeleteQuery()) {
       result = rowCount;
-    } else if (this.isInsertQuery() || this.isUpdateQuery()) {
-      result = [result, rowCount];
     } else if (this.isShowConstraintsQuery()) {
       result = data;
     } else if (this.isRawQuery()) {
@@ -406,26 +429,4 @@ export class Db2Query extends AbstractQuery {
 
     return Array.from(indexes.values());
   }
-
-  handleInsertQuery(results, metaData) {
-    if (!this.instance) {
-      return;
-    }
-
-    const modelDefinition = this.model.modelDefinition;
-    if (!modelDefinition.autoIncrementAttributeName) {
-      return;
-    }
-
-    const autoIncrementAttribute = modelDefinition.attributes.get(
-      modelDefinition.autoIncrementAttributeName,
-    );
-
-    const id =
-      results?.[0][this.getInsertIdField()] ??
-      metaData?.[this.getInsertIdField()] ??
-      results?.[0][autoIncrementAttribute.columnName];
-
-    this.instance[autoIncrementAttribute.attributeName] = id;
-  }
 }
diff --git a/packages/core/src/dialects/mssql/query.js b/packages/core/src/dialects/mssql/query.js
@@ -1,7 +1,5 @@
 'use strict';
 
-import { getAttributeName } from '../../utils/format';
-
 import forOwn from 'lodash/forOwn';
 import zipObject from 'lodash/zipObject';
 
@@ -165,10 +163,45 @@ export class MsSqlQuery extends AbstractQuery {
    *  ])
    */
   formatResults(data, rowCount) {
-    if (this.isInsertQuery(data)) {
-      this.handleInsertQuery(data);
+    if (this.isInsertQuery() || this.isUpdateQuery() || this.isUpsertQuery()) {
+      if (this.instance && this.instance.dataValues) {
+        // If we are creating an instance, and we get no rows, the create failed but did not throw.
+        // This probably means a conflict happened and was ignored, to avoid breaking a transaction.
+        if (this.isInsertQuery() && !this.isUpsertQuery() && data.length === 0) {
+          throw new sequelizeErrors.EmptyResultError();
+        }
+
+        // if this was an upsert and no data came back, that means the record exists, but the update was a noop.
+        // return the current instance and mark it as an "not an insert".
+        if (this.isUpsertQuery() && data.length === 0) {
+          return [this.instance || data, false];
+        }
+
+        if (Array.isArray(data) && data[0]) {
+          for (const attributeOrColumnName of Object.keys(data[0])) {
+            const modelDefinition = this.model.modelDefinition;
+            const attribute = modelDefinition.columns.get(attributeOrColumnName);
+            const updatedValue = this._parseDatabaseValue(
+              data[0][attributeOrColumnName],
+              attribute?.type,
+            );
+
+            this.instance.set(attribute?.attributeName ?? attributeOrColumnName, updatedValue, {
+              raw: true,
+              comesFromDatabase: true,
+            });
+          }
+        }
+      }
+
+      if (this.isUpsertQuery()) {
+        return [this.instance || data, data[0].$action === 'INSERT'];
+      }
 
-      return [this.instance || data, rowCount];
+      return [
+        this.instance || (data && ((this.options.plain && data[0]) || data)) || undefined,
+        rowCount,
+      ];
     }
 
     if (this.isDescribeQuery()) {
@@ -212,33 +245,13 @@ export class MsSqlQuery extends AbstractQuery {
     }
 
     if (this.isBulkUpdateQuery()) {
-      if (this.options.returning) {
-        return this.handleSelectQuery(data);
-      }
-
-      return rowCount;
+      return this.options.returning ? this.handleSelectQuery(data) : rowCount;
     }
 
     if (this.isDeleteQuery()) {
       return data[0] ? data[0].AFFECTEDROWS : 0;
     }
 
-    if (this.isUpsertQuery()) {
-      // if this was an upsert and no data came back, that means the record exists, but the update was a noop.
-      // return the current instance and mark it as an "not an insert".
-      if (data && data.length === 0) {
-        return [this.instance || data, false];
-      }
-
-      this.handleInsertQuery(data);
-
-      return [this.instance || data, data[0].$action === 'INSERT'];
-    }
-
-    if (this.isUpdateQuery()) {
-      return [this.instance || data, rowCount];
-    }
-
     if (this.isShowConstraintsQuery()) {
       return data;
     }
@@ -413,38 +426,4 @@ export class MsSqlQuery extends AbstractQuery {
 
     return Array.from(indexes.values());
   }
-
-  handleInsertQuery(insertedRows, metaData) {
-    if (!this.instance?.dataValues) {
-      return;
-    }
-
-    // map column names to attribute names
-    insertedRows = insertedRows.map(row => {
-      const attributes = Object.create(null);
-
-      for (const columnName of Object.keys(row)) {
-        const attributeName = getAttributeName(this.model, columnName) ?? columnName;
-
-        attributes[attributeName] = row[columnName];
-      }
-
-      return attributes;
-    });
-
-    insertedRows = this._parseDataArrayByType(insertedRows, this.model, this.options.includeMap);
-
-    const autoIncrementAttributeName = this.model.autoIncrementAttribute;
-    let id = null;
-
-    id ||= insertedRows && insertedRows[0][this.getInsertIdField()];
-    id ||= metaData && metaData[this.getInsertIdField()];
-    id ||= insertedRows && insertedRows[0][autoIncrementAttributeName];
-
-    // assign values to existing instance
-    this.instance[autoIncrementAttributeName] = id;
-    for (const attributeName of Object.keys(insertedRows[0])) {
-      this.instance.dataValues[attributeName] = insertedRows[0][attributeName];
-    }
-  }
 }
diff --git a/packages/core/src/dialects/postgres/query.js b/packages/core/src/dialects/postgres/query.js
@@ -280,21 +280,16 @@ export class PostgresQuery extends AbstractQuery {
           throw new sequelizeErrors.EmptyResultError();
         }
 
-        if (rows[0]) {
+        if (Array.isArray(rows) && rows[0]) {
           for (const attributeOrColumnName of Object.keys(rows[0])) {
             const modelDefinition = this.model.modelDefinition;
-
-            // TODO: this should not be searching in both column names & attribute names. It will lead to collisions. Use only one or the other.
-            const attribute =
-              modelDefinition.attributes.get(attributeOrColumnName) ??
-              modelDefinition.columns.get(attributeOrColumnName);
-
+            const attribute = modelDefinition.columns.get(attributeOrColumnName);
             const updatedValue = this._parseDatabaseValue(
               rows[0][attributeOrColumnName],
               attribute?.type,
             );
 
-            this.instance.set(attribute?.fieldName ?? attributeOrColumnName, updatedValue, {
+            this.instance.set(attribute?.attributeName ?? attributeOrColumnName, updatedValue, {
               raw: true,
               comesFromDatabase: true,
             });