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

toJSON converting booleans to "t" (true) or "f" (false) #8749

Closed
jonahlau opened this issue Dec 4, 2017 · 18 comments
Closed

toJSON converting booleans to "t" (true) or "f" (false) #8749

jonahlau opened this issue Dec 4, 2017 · 18 comments
Labels
dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). type: bug

Comments

@jonahlau
Copy link

jonahlau commented Dec 4, 2017

What are you doing?

I have the user model as defined below.

// User model

sequelize.define('user', {
  email: {
    type: STRING,
    allowNull: false,
    unique: true,
    validate: {
      isEmail: true,
      notEmpty: true
    }
  },
  password: {
    type: STRING,
    allowNull: false,
    visible: []
  },
  firstName: {
    type: STRING,
    allowNull: false
  },
  lastName: {
    type: STRING,
    allowNull: false
  },
  active: {
    type: BOOLEAN,
    allowNull: false,
    defaultValue: true
  },
  role: {
    type: STRING,
    allowNull: false,
    defaultValue: 'user'
  },
  isVerified: {
    type: BOOLEAN,
    defaultValue: false
  },
  resetPasswordToken: {
    type: STRING,
    allowNull: true,
    visible: []
  },
  resetPasswordExpires: {
    type: DATE,
    allowNull: true,
    visible: []
  },
  lastLogoutTime: {
    type: DATE,
    allowNull: true,
    visible: ['admin']
  }
});

What do you expect to happen?

toJSON should retain booleans and not convert them to strings

What is actually happening?

When I call toJSON of any instance of User in my CI tests (running in as a docker container with node 8.9.1), all booleans, for example fields active and isVerified, get converted to "t" or "f".

The problem doesn't happen when I run it on MacOS 10.12.15 running Node 8.9.1

Output, either JSON or SQL

Dialect: postgres
Dialect version: XXX
Database version: 9.5
Sequelize version: 4.2.0
Tested with latest release: yes (If yes, specify that version)

Note : Your issue may be ignored OR closed by maintainers if it's not tested against latest version OR does not follow issue template.

@karn09
Copy link

karn09 commented Dec 4, 2017

I'm seeing this same issue, but with 4.26.0. @jonahlau Did you mean 4.26.0 as well? You had marked yes to tested with latest.

Downgrading to 4.25.2 fixed the issue for me.

@jonahlau
Copy link
Author

jonahlau commented Dec 5, 2017

@karn09 Ah, yes sorry for the typo. I did mean 4.26.0

Downgrading to 4.25.2 fixed it for me. So should I avoid upgrading to latest release until this is resolved?

@karn09
Copy link

karn09 commented Dec 5, 2017

@jonahlau until it's fixed probably a good idea, unless you need the fixes in the newer version more then proper booleans on toJSON.

@dee-el
Copy link

dee-el commented Dec 6, 2017

I seeing same issue with
Nodejs version : v8.1.4
OS version : Ubuntu 16.04
DB dialect: postgress

@tapz
Copy link

tapz commented Dec 11, 2017

4.28.0 still has this issue.

@sushantdhiman
Copy link
Contributor

FYI I tried to replicate this issue but failed, #8791 (test case)

If some one can add a reproducible test case I will take care of fixing it

@aaltepet
Copy link

aaltepet commented Dec 19, 2017

I don't know whether it's possible to have a test case for this. What I do know is that when you connect the latest sequelize releases to a postgres database, and query a model with a boolean column, the values come back as 't' and 'f'.

The most recent release that correctly returns bools from postgres is 4.25.2. Perhaps something introduced with postgres support for enums broke the bool typecasting?

With release 4.26.0 (and 4.28.6), I have a model A with a boolean property prop. The returned instance from A.create(...) has prop='t', whereas reloading causes the value to be properly casted.

const instance = await A.create({ prop: true }); instance.dataValues.prop === 't'; await instance.reload(); instance.dataValues.prop === true

@tapz
Copy link

tapz commented Dec 19, 2017

't' probably is what Postgre returns. There seems to be no conversion to JS boolean from the string value returned by Postgre.

@sushantdhiman
Copy link
Contributor

But you guys are facing this issue consistently on at least one machine, lets start with that.

Please share your setup details (as much as possible) and answer these questions

  • OS version
  • Node version (Running inside docker, state image name and tag)
  • Full Postgres DB version [is it a docker image (specify image name and tag) ]
  • Any extensions on Postgres DB (List them, with version number if available)
  • Do you have any ENUM defined in your Postgres DB?
  • Do you see this issue on isolated script running against same DB container Or just in your project code?

@tapz
Copy link

tapz commented Dec 20, 2017

Our system has some unit tests, which produce this issue every time when run with 4.28.6. I run the tests on my own machine.

MacOS 10.13.2 (17C88)
Node 8.9.3
Postgres 9.6.2
pg 6.4.2
pg-native 2.2.0 (not sure if really used)

We do have some ENUMs defined, but not in the same table where the booleans do not work.

@aaltepet
Copy link

Our system has some unit tests which produce this issue when run with 4.28.6. We run the tests within a docker-compose environment.

Node image: node:8.9.0
Postgres image: postgres:9.6.1
sequelize: 4.28.6
pg: 6.1.2

To narrow down the test, I did the following in my environment:

created a new model:

module.exports = function (sequelize, DataTypes) {
    const BoolModel = sequelize.define('boolModel', {
        flag: { type: DataTypes.BOOLEAN, allowNull: false, defaultValue: true }
    });
    return BoolModel;
};

Created a mocha/chai test to verify behavior:

const models = require('../db').models;
const expect = require('chai').expect;

describe('bool tests', function () {
    it('returns a string instead of a bool', async function () {
        const boolModel = await models.BoolModel.create();
        expect(boolModel.flag).to.equal('t');
        await boolModel.reload();
        expect(boolModel.flag).to.equal(true);
    });
});

Output from mocha:

  bool tests
    ✓ returns a string instead of a bool

@rokoroku
Copy link
Contributor

Same here

  • Node 8.9.0
  • Postgres 9.6.2
  • pg 6.4.2
  • seqeulize 4.28.6

Even creating a new model with default valued boolean fields, it returns 't' or 'f', exactly like
above example by @aaltepet

And downgrading to 4.25.2 fixed it temporarily.

@kapvode
Copy link

kapvode commented Jan 2, 2018

I noticed this problem when doing finds with { raw: true }, instead of calling toJSON() explicitly.

I struggled to reproduce it in an isolated example, but your comments about ENUMs have helped me and I now have something that can reliably reproduce it for me. It only happens when I define an ENUM field.

These are my versions:

  • Node 8.9.3
  • Postgres 9.6.6
  • sequelize 4.28.6
  • pg 6.4.2
'use strict';

const Sequelize = require('sequelize');
const Op = Sequelize.Op;

let Example;

/**
 * @param {boolean} useEnum
 */
async function test(useEnum) {
  const db = new Sequelize('test', 'user', 'user', {
    host: 'postgres',
    dialect: 'postgres',
    operatorsAliases: false,
  });

  const attributes = {
    s: { type: Sequelize.STRING, allowNull: false },
    b: { type: Sequelize.BOOLEAN, allowNull: false },
  };
  if (useEnum) {
    attributes.e = {
      type: Sequelize.ENUM,
      values: ['one', 'two', 'three'],
      defaultValue: 'two',
    };
  }
  const options = {
    freezeTableName: true,
    indexes: [
      { fields: ['s'], unique: true },
    ],
  };

  Example = db.define('example', attributes, options);

  await db.sync({ force: true });
  await populateDb();
  await findOne();
  await db.close();
}

async function populateDb() {
  const e = await Example.create({ s: 'text', b: true });
  logText('Create');
  logModel(e);
}

async function findOne() {
  const e1 = await Example.findOne({ where: { s: { [Op.eq]: 'text' } } });
  logText('Normal find');
  logModel(e1);

  const e2 = await Example.findOne({ where: { s: { [Op.eq]: 'text' } }, raw: true });
  logText('Raw find');
  logModel(e2);
}

function logHeading() {
  process.stdout.write("\x1b[34m");
  console.log(...arguments);
  process.stdout.write("\x1b[0m");
}

function logText() {
  process.stdout.write("\x1b[32m");
  console.log(...arguments);
  process.stdout.write("\x1b[0m");
}

function logModel(e) {
  process.stdout.write("\x1b[33m");
  console.log(e.s, e.b, e.e);
  process.stdout.write("\x1b[0m");
}

async function main() {
  try {
    logHeading('Model with no enums');
    await test();
    logHeading('Model with enums');
    await test(true);
  } catch (err) {
    console.error(err);
    process.exit(1);
  }
}

main();

@themetalfleece
Copy link

I'm facing the same problem @4.28.6 (postgres) when running the following.

return rental.increment(
 'quantity',
 { by: 1, returning: true, transaction: t }
)

the returned object has 't' or 'f' instead of booleans

@emhagman
Copy link

emhagman commented Jan 5, 2018

+1 Having this issue, our tests caught this.

{ 
  id: 1,
  active: 'f', // THIS IS BOOLEAN in PSQL and Model
  unverified_email: 'bob@example.us',
  emails: []
}
Sequelize: 4.28.6
Postgresql: 9.6
NodeJS: 9.3
Docker Image: node9.3 (Debian)

Happens when using { raw: true } on the findOne()

Can confirm that Sequelize 4.25.2 fixes the test as well.

@sushantdhiman
Copy link
Contributor

Thanks @kapvode for a proper test case. As suspected a model with ENUM fetch OIDs properly but wont refresh base types like BOOLEAN in this case.

Everyone please use #8851 patch and verify if that fix this issue. After verification and review from other maintainers I will merge that PR.

@sushantdhiman sushantdhiman added the dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). label Jan 7, 2018
@tapz
Copy link

tapz commented Jan 7, 2018

Well done @sushantdhiman, the fix seems to work with my test cases!

@kapvode
Copy link

kapvode commented Jan 8, 2018

It works for me too. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialect: postgres For issues and PRs. Things that involve PostgreSQL (and do not involve all dialects). type: bug
Projects
None yet
Development

No branches or pull requests

10 participants