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

refactor(scan.go): array index is out of bounds exits the for loop #6881

Closed
wants to merge 5 commits into from

Conversation

demoManito
Copy link
Member

@demoManito demoManito commented Mar 7, 2024

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

array is passed in, and the number of rows returned exceeds the length of the array, ending the current scan.

User Case Description

@demoManito demoManito force-pushed the refactor/scan-array branch from 41da0d4 to 1b9d1ad Compare March 17, 2024 15:13
@jinzhu
Copy link
Member

jinzhu commented Mar 18, 2024

Hi @demoManito

Thank you for your PR, can you add some tests?

@demoManito demoManito force-pushed the refactor/scan-array branch from 1b9d1ad to 2b8fd9d Compare March 18, 2024 05:59
@demoManito
Copy link
Member Author

Hi @demoManito

Thank you for your PR, can you add some tests?

done @jinzhu

@jinzhu
Copy link
Member

jinzhu commented Mar 18, 2024

tests pass w/o those changes?

@jinzhu
Copy link
Member

jinzhu commented Mar 18, 2024

tests pass w/o those changes?

Ah, apologies for the oversight on my part regarding the pull request.

It appears this submission is aimed at refactoring rather than bug fixing.

Nonetheless, considering the usage of array types tends to be an edge case, maintaining the existing approach might be preferable to avoid introducing extra checks for most scenarios.

Thank you anyway.

@jinzhu jinzhu closed this Mar 18, 2024
@jinzhu jinzhu deleted the refactor/scan-array branch March 18, 2024 11:39
@demoManito
Copy link
Member Author

tests pass w/o those changes?

Ah, apologies for the oversight on my part regarding the pull request.

It appears this submission is aimed at refactoring rather than bug fixing.

Nonetheless, considering the usage of array types tends to be an edge case, maintaining the existing approach might be preferable to avoid introducing extra checks for most scenarios.

Thank you anyway.

Okay, I understand

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

Successfully merging this pull request may close these issues.

2 participants