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

misleading documentation on "type" parameter #294

Open
runfaj opened this issue Dec 12, 2018 · 2 comments
Open

misleading documentation on "type" parameter #294

runfaj opened this issue Dec 12, 2018 · 2 comments

Comments

@runfaj
Copy link

runfaj commented Dec 12, 2018

https://github.com/sequelize/sequelize/blob/ff1c97cd9264d50c1d779b31a8d38dd4182ddfaa/lib/sequelize.js#L427

This documentation indicates that ANY time the "type" parameter is specified, the returned result is the row, without the counts. However, looking at the actual code:

https://github.com/sequelize/sequelize/blob/ff1c97cd9264d50c1d779b31a8d38dd4182ddfaa/lib/dialects/postgres/query.js#L261

This is NOT the case with insert/update/raw. This should either be adjusted in the documentation to mention excluded cases, or should be updated in the Query to return the expected result for these other cases. I vote the latter personally, as it provides more code flexibility in general - but I do understand that would be a breaking change, since people would have had to work around this issue by now.

It should also be noted, that this was working properly in sequelize v3. When I started migrating to v4, this issue presented itself. Here's the example query I had to modify as a result:

   return sequelize.query(sql, {
        type: models.Sequelize.QueryTypes.UPDATE,
        replacements: {
          jobId,
          status: metricStatuses.PROCESSING,
          workerMeta: JSON.stringify(workerMeta),
        },
        raw: true,
      })
      .then(_.first) ////////// this shouldn't be needed, docs say setting the type avoids this
      .then(_.first);
@stale
Copy link

stale bot commented Mar 13, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment 🙂

@stale stale bot added the stale label Mar 13, 2019
@stale
Copy link

stale bot commented Jul 23, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment 🙂

@stale stale bot added the stale label Jul 23, 2019
@papb papb removed the stale label Jul 24, 2019
@ephys ephys transferred this issue from sequelize/sequelize Oct 6, 2022
@ephys ephys removed the type: docs label Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants