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

fix(oracle): remove hardcoded maxRows value #15323

Merged
merged 2 commits into from Nov 22, 2022

Conversation

nilabja-bhattacharya
Copy link

@nilabja-bhattacharya nilabja-bhattacharya commented Nov 22, 2022

Pull Request Checklist

  • Have you added new tests to prevent regressions?
  • If a documentation update is necessary, have you opened a PR to the documentation repository?
  • Did you update the typescript typings accordingly (if applicable)?
  • Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
  • Does the name of your PR follow our conventions?

Description Of Change

The Oracle dialect had the maxRows value hardcoded to 1000 so select queries returned only the first 1000 rows unless the user set the maxRows option. Fixing this would fetch all the rows from the table instead.

Fix to the issue raised in https://stackoverflow.com/questions/74357044/sequelize-have-a-limit-of-return-1000-rows-of-my-table-oracle-connection

@fzn0x fzn0x requested a review from ephys November 22, 2022 05:53
@cjbj
Copy link

cjbj commented Nov 22, 2022

The very original node-oracledb versions needed an upper bound set. This is no longer true, and we improved internal row buffering, so it makes sense to fetch everything by default.

@WikiRik WikiRik changed the title Fix: Removed hardcoded maxRows value in the Oracle dialect fix(oracle): remove hardcoded maxRows value Nov 22, 2022
Copy link
Member

@WikiRik WikiRik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! The test that was failing is flaky so I've rerun it

@WikiRik WikiRik merged commit 7885000 into sequelize:v6 Nov 22, 2022
@github-actions
Copy link
Contributor

🎉 This PR is included in version 6.25.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants