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.

Sign up for GitHub

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

Jump to bottom

result: fix wrong implied lifetime in col_specs() #1109

Merged
merged 1 commit into from
Oct 28, 2024
Merged

result: fix wrong implied lifetime in col_specs() #1109

merged 1 commit into from
Oct 28, 2024

Conversation

wprzytula
Copy link
Collaborator

As ColumnSpec is now parametrised by a lifetime, methods that return it implicitly bound its lifetime with lifetime of &self. This bug would lead to inefficiencies.
As a fix, an explicit lifetime is added in every method returning ColumnSpec.

Ref: scylladb/cpp-rust-driver#200 (comment)

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • [] I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • [ ] I have provided docstrings for the public items that I want to introduce.
  • [ ] I have adjusted the documentation in ./docs/source/.
  • [ ] I added appropriate Fixes: annotations to PR description.

@wprzytula wprzytula requested a review from muzarski October 28, 2024 14:20
@wprzytula wprzytula self-assigned this Oct 28, 2024
@wprzytula wprzytula added the bug Something isn't working label Oct 28, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 28, 2024 •
edited
Loading

cargo semver-checks found no API-breaking changes in this PR! 🎉🥳
Checked commit: 8dd1498

@muzarski
Copy link
Contributor

Following should be adjusted as well:

  • TypedRowIterator::get_column_specs
  • PreparedStatement::get_variable_col_specs
  • PreparedStatement::get_result_set_col_specs

@wprzytula
result: fix wrong implied lifetime in col_specs()
8dd1498 
As `ColumnSpec` is now parametrised by a lifetime, methods that return
it implicitly bound its lifetime with lifetime of `&self`. This bug
would lead to inefficiencies.
As a fix, an explicit lifetime is added in every method returning
`ColumnSpec`.
@wprzytula wprzytula force-pushed the col-specs-fix-lifetime branch from 8120d2f to 8dd1498 Compare October 28, 2024 14:37
muzarski
muzarski approved these changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@muzarski muzarski left a comment

Choose a reason for hiding this comment

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

Thanks!

@wprzytula wprzytula requested a review from Lorak-mmk October 28, 2024 14:41
@Lorak-mmk Lorak-mmk merged commit da199a1 into scylladb:main Oct 28, 2024
11 checks passed
@wprzytula wprzytula deleted the col-specs-fix-lifetime branch October 28, 2024 14:48
@wprzytula wprzytula mentioned this pull request Nov 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@muzarski muzarski muzarski approved these changes

@Lorak-mmk Lorak-mmk Lorak-mmk approved these changes

Assignees

@wprzytula wprzytula

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wprzytula @muzarski @Lorak-mmk