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

constructor options: have a default limit for queries #12648

Open
2 of 6 tasks
mariusa opened this issue Aug 27, 2020 · 6 comments
Open
2 of 6 tasks

constructor options: have a default limit for queries #12648

mariusa opened this issue Aug 27, 2020 · 6 comments
Labels
existing workaround For issues. There is a known workaround for this issue. type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@mariusa
Copy link

mariusa commented Aug 27, 2020

Feature Description

Use case: we want to LIMIT all SQL SELECTs to a maximum number of records, eg 500.
This limit can still be set for every query, eg to handle pagination, but if not set, it should have a default.

Describe the solution you'd like

https://sequelize.org/master/class/lib/sequelize.js~Sequelize.html#instance-constructor-constructor
says
options.query | object | optionaldefault: {} | Default options for sequelize.query

but this

new Sequelize(process.env.DATABASE_URL, {
        logging: false,
        query: {
             limit: 500
        }
    })

has no effect.

Why should this be in Sequelize

We use Sequelize to read data.

Describe alternatives/workarounds you've considered

This works

model.addScope('defaultScope', {
            limit: DB_PAGE_SIZE
        })

but has to be done for every model. It would be simpler to have a default for sequelize.

Feature Request Checklist

Is this feature dialect-specific?

  • No. This issue is relevant to Sequelize as a whole.
  • Yes. This issue only applies to the following dialect(s): XXX, YYY, ZZZ

Would you be willing to implement this feature by submitting a Pull Request?

  • Yes, I have the time and I know how to start.
  • Yes, I have the time but I don't know how to start, I would need guidance.
  • No, I don't have the time, although I believe I could do it if I had the time...
  • No, I don't have the time and I wouldn't even know how to start.
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has been open for 7 days without activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

@github-actions github-actions bot added the stale label Oct 30, 2021
@mariusa
Copy link
Author

mariusa commented Oct 31, 2021

ping

@github-actions github-actions bot removed the stale label Nov 1, 2021
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has been open for 14 days without activity. It will be closed if no further activity occurs within the next 14 days. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

@github-actions github-actions bot added the stale label Nov 16, 2021
@mariusa
Copy link
Author

mariusa commented Nov 16, 2021

...

@WikiRik WikiRik added type: feature For issues and PRs. For new features. Never breaking changes. existing workaround For issues. There is a known workaround for this issue. and removed stale labels Nov 16, 2021
@fesaza
Copy link

fesaza commented Nov 18, 2021

Would be very useful

@enes-sahin
Copy link

Another workaround is to add following code to models/index.js in forEachloop:

 model.findAll = function (options) {
    return Sequelize.Model.findAll.call(model, { limit: 25, ...options });
  };

So models/index.js looks like the following:

'use strict';

const fs = require('fs');
const path = require('path');
const Sequelize = require('sequelize');
const basename = path.basename(__filename);
const env = process.env.NODE_ENV || 'development';
let config = require(`${__dirname}/../config/config.js`)[env];
const db = {};

let sequelize;
if (config.use_env_variable) {
  sequelize = new Sequelize(process.env[config.use_env_variable], config);
} else {
  sequelize = new Sequelize(config.database, config.username, config.password, config);
}

fs.readdirSync(__dirname)
  .filter((file) => {
    return file.indexOf('.') !== 0 && file !== basename && file.slice(-3) === '.js';
  })
  .forEach((file) => {
    const model = require(path.join(__dirname, file))(sequelize, Sequelize.DataTypes);

    model.findAll = function (options) {
      return Sequelize.Model.findAll.call(model, { limit: 25, ...options });
    };

    db[model.name] = model;
  });

Object.keys(db).forEach((modelName) => {
  if (db[modelName].associate) {
    db[modelName].associate(db);
  }
});

db.sequelize = sequelize;
db.Sequelize = Sequelize;

module.exports = db;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
existing workaround For issues. There is a known workaround for this issue. type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

No branches or pull requests

4 participants