Merge pull request #420 from strongloop/fixbuildwhere

fix: `DEFAULT` for null values in where clause

Author: deepakrkris

lib/postgresql.js

@@ -644,10 +644,10 @@ PostgreSQL.prototype._buildWhere = function(model, where) {
         if (Array.isArray(expression)) {
           // Column value is a list
           for (let j = 0, m = expression.length; j < m; j++) {
-            columnValue.push(this.toColumnValue(p, expression[j]));
+            columnValue.push(this.toColumnValue(p, expression[j], true));
           }
         } else {
-          columnValue.push(this.toColumnValue(p, expression));
+          columnValue.push(this.toColumnValue(p, expression, true));
         }
         if (operator === 'between') {
           // BETWEEN v1 AND v2
@@ -669,7 +669,7 @@ PostgreSQL.prototype._buildWhere = function(model, where) {
         // do not coerce RegExp based on property definitions
         columnValue = expression;
       } else {
-        columnValue = this.toColumnValue(p, expression);
+        columnValue = this.toColumnValue(p, expression, true);
       }
       sqlExp = self.buildExpression(columnName, operator, columnValue, p);
       stmt.merge(sqlExp);
@@ -711,13 +711,15 @@ PostgreSQL.prototype._buildWhere = function(model, where) {
  * Convert property name/value to an escaped DB column value
  * @param {Object} prop Property descriptor
  * @param {*} val Property value
+ * @param {boolean} isWhereClause
  * @returns {*} The escaped value of DB column
  */
-PostgreSQL.prototype.toColumnValue = function(prop, val) {
+PostgreSQL.prototype.toColumnValue = function(prop, val, isWhereClause) {
   if (val == null) {
     // PostgreSQL complains with NULLs in not null columns
     // If we have an autoincrement value, return DEFAULT instead
-    if (prop.autoIncrement || prop.id) {
+    // Do not return 'DEFAULT' for id field in where clause
+    if (prop.autoIncrement || (prop.id && !isWhereClause)) {
       return new ParameterizedSQL('DEFAULT');
     } else {
       return null;

test/postgresql.filterUndefined.test.js

@@ -6,7 +6,7 @@
 'use strict';
 const should = require('should'),
   assert = require('assert');
-let Post, db;
+let Post, db, Charge, Currency;
 
 describe('filter undefined fields', function() {
   before(function() {
@@ -136,4 +136,55 @@ describe('filter undefined fields', function() {
       });
     });
   });
+
+  describe('able to handle null foreign keys', function() {
+    before(function(done) {
+      Charge = db.define('Charge', {amount: Number});
+      Currency = db.define('Currency', {code: String, country: String});
+      Charge.belongsTo('Currency', {as: 'currency'});
+      db.automigrate(['Charge', 'Currency'], done);
+    });
+
+    it('able to create entity with null foreign key', function(done) {
+      Currency.create({code: 'USD', country: 'USA'}, function(err, currency) {
+        if (err) return done(err);
+        Charge.create({amount: 10, currencyId: currency.id}, function(err) {
+          if (err) return done(err);
+          Charge.create({amount: 10, currencyId: null}, function(err, charge) {
+            if (err) return done(err);
+            should.not.exist(charge.currencyId);
+            done();
+          });
+        });
+      });
+    });
+
+    it('able to query entities with null foreign key', function(done) {
+      Charge.find({
+        where: {currency: {
+          inq: [null, 1],
+        }},
+        include: {relation: 'currency'},
+      }, function(err, charges) {
+        if (err) return done(err);
+        charges.should.have.lengthOf(2);
+        done();
+      });
+    });
+
+    it('able to filter entities with null id condition', function(done) {
+      Currency.find({
+        where: {
+          id: {
+            inq: [null, 1],
+          },
+        },
+      },
+      function(err, currencies) {
+        if (err) return done(err);
+        currencies.should.have.lengthOf(1);
+        done();
+      });
+    });
+  });
 });