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

Improve digest_to_checksum for SQL Server #97

Merged
merged 16 commits into from
Feb 2, 2024

Conversation

RasmusSkytte
Copy link
Contributor

@RasmusSkytte RasmusSkytte commented Feb 1, 2024

Intent

This PR add a specific implementation of digest_to_checksum for SQL server which speeds up the computation of checksums on the backend.
(Most of these updates are taken from @marcusmunch's fork)

Approach

A S3 method is added for SQL Server connections which uses the built-in HashBytes to compute checksums with SHA_256 instead of copying the data to the local session and using the R openssl::md5 algorithm.

Known issues

This PR also contains the code being merged in

Once merged, this PR will be rebased and opened.

Checklist

  • The PR passes all local unit tests
  • I have documented any new features introduced
  • If the PR adds a new feature, please add an entry in NEWS.md
  • A reviewer is assigned to this PR

@RasmusSkytte RasmusSkytte marked this pull request as draft February 1, 2024 09:50
@RasmusSkytte RasmusSkytte self-assigned this Feb 1, 2024
@RasmusSkytte RasmusSkytte force-pushed the feature/digest_to_checksum_mssql branch from 2200800 to 94a72dc Compare February 1, 2024 14:08
@RasmusSkytte RasmusSkytte added the enhancement New feature or request label Feb 1, 2024
@RasmusSkytte RasmusSkytte marked this pull request as ready for review February 1, 2024 14:23
@RasmusSkytte RasmusSkytte requested review from a team, kaare-gr and LasseEngboChr and removed request for a team February 1, 2024 14:23
NEWS.md Outdated Show resolved Hide resolved
R/update_snapshot.R Outdated Show resolved Hide resolved
R/update_snapshot.R Outdated Show resolved Hide resolved
Copy link
Contributor

@LasseEngboChr LasseEngboChr left a comment

Choose a reason for hiding this comment

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

See comments

Co-authored-by: Lasse Engbo Christiansen <147731649+LasseEngboChr@users.noreply.github.com>
@RasmusSkytte RasmusSkytte merged commit c0223cc into main Feb 2, 2024
24 checks passed
@RasmusSkytte RasmusSkytte deleted the feature/digest_to_checksum_mssql branch February 2, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants