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 regexp_is_match_utf8_scalar to try to mitigate miri failures #895

Merged
merged 1 commit into from Nov 1, 2021

Conversation

jimexist
Copy link
Member

@jimexist jimexist commented Nov 1, 2021

Which issue does this PR close?

Closes #879

Rationale for this change

refactor regexp_is_match_utf8_scalar to try to mitigate miri failures

What changes are included in this PR?

refactor regexp_is_match_utf8_scalar to try to mitigate miri failures

Are there any user-facing changes?

@github-actions github-actions bot added the arrow Changes to the arrow crate label Nov 1, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #895 (b1a10e0) into master (06f730e) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #895      +/-   ##
==========================================
- Coverage   82.30%   82.30%   -0.01%     
==========================================
  Files         168      168              
  Lines       48031    48031              
==========================================
- Hits        39533    39532       -1     
- Misses       8498     8499       +1     
Impacted Files Coverage Δ
arrow/src/compute/kernels/comparison.rs 92.63% <100.00%> (ø)
arrow/src/datatypes/datatype.rs 64.93% <0.00%> (-0.44%) ⬇️
parquet_derive/src/parquet_field.rs 66.21% <0.00%> (-0.23%) ⬇️
arrow/src/array/transform/mod.rs 85.33% <0.00%> (+0.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 06f730e...b1a10e0. Read the comment docs.

@jimexist jimexist changed the title test moving out refactor regexp_is_match_utf8_scalar to try to mitigate miri failures Nov 1, 2021
@jimexist jimexist marked this pull request as ready for review November 1, 2021 04:46
@jimexist jimexist requested a review from alamb November 1, 2021 04:46
@jimexist jimexist changed the title refactor regexp_is_match_utf8_scalar to try to mitigate miri failures refactor regexp_is_match_utf8_scalar to try to mitigate miri failures Nov 1, 2021
@@ -597,14 +595,15 @@ pub fn regexp_is_match_utf8_scalar<OffsetSize: StringOffsetSizeTrait>(
}
}

let buffer = result.finish();
Copy link
Member Author

Choose a reason for hiding this comment

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

not sure if moving this out would help but let me try this anyway

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

This seems like a reasonable code change to me, though I don't think MIRI is failing anymore on master, is it?

https://github.com/apache/arrow-rs/runs/4064195444?check_suite_focus=true

@jimexist jimexist merged commit a7547d5 into apache:master Nov 1, 2021
@jimexist jimexist deleted the test-move-out branch November 1, 2021 10:44
@jimexist
Copy link
Member Author

jimexist commented Nov 1, 2021

This seems like a reasonable code change to me, though I don't think MIRI is failing anymore on master, is it?

https://github.com/apache/arrow-rs/runs/4064195444?check_suite_focus=true

I did a retry so not sure why it was fixed I guess I'll merge this as is and see if it resurfaces

alamb pushed a commit that referenced this pull request Nov 9, 2021
@alamb alamb added the cherry-picked PR that was backported to active release (will be included in maintenance release) label Nov 9, 2021
alamb added a commit that referenced this pull request Nov 9, 2021
Co-authored-by: Jiayu Liu <Jimexist@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate cherry-picked PR that was backported to active release (will be included in maintenance release)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MIRI check is failing on master
3 participants