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

Enable casting from Utf8View #6077

Merged
merged 5 commits into from
Jul 19, 2024
Merged

Conversation

a10y
Copy link
Contributor

@a10y a10y commented Jul 17, 2024

Which issue does this PR close?

Closes #6076

Rationale for this change

See issue

What changes are included in this PR?

Mirroring of the existing Utf8 cast operations for Utf8View

Are there any user-facing changes?

Purely additive, this adds functionality for users that have Utf8View columns.

@github-actions github-actions bot added the arrow Changes to the arrow crate label Jul 17, 2024
parse_string_iter::<P, _, _>(string_view_array.iter(), cast_options, || string_view_array.nulls().cloned())
}

fn parse_string_iter<'a, P: Parser, I: Iterator<Item=Option<&'a str>>, F: FnOnce() -> Option<NullBuffer>>(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wanted to extract out the common functionality, could also be a macro if preferred

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can use this trait: https://github.com/apache/arrow-rs/blob/master/arrow-string/src/like.rs#L158? Probably need to make it a public trait first.

@a10y a10y marked this pull request as ready for review July 18, 2024 15:41
@a10y
Copy link
Contributor Author

a10y commented Jul 18, 2024

This is ready for review, I've run cargo test -p arrow-cast locally and everything passes

@a10y
Copy link
Contributor Author

a10y commented Jul 18, 2024

Hm, I'm not sure what's up with the archery test, I'm seeing some connection denied after 10 minutes, I don't have permission to retrigger the check but I'm wondering if it's possibly a flake?

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.

Thank you @a10y. I took the liberty of running cargo fmt and pushing a new commit to this branch to get the CI to pass

@alamb
Copy link
Contributor

alamb commented Jul 19, 2024

Hm, I'm not sure what's up with the archery test, I'm seeing some connection denied after 10 minutes, I don't have permission to retrigger the check but I'm wondering if it's possibly a flake?

Yes, I think it was a flake. Also once we have merged this PR I think your subsequent PRs will automatically start CI so we should be able to reduce the turnaround time on future PRs

@alamb alamb merged commit 8a5be13 into apache:master Jul 19, 2024
25 checks passed
@alamb
Copy link
Contributor

alamb commented Jul 19, 2024

Thanks again @a10y

@a10y
Copy link
Contributor Author

a10y commented Jul 19, 2024

Thanks for the quick reviews!

@alamb
Copy link
Contributor

alamb commented Jul 19, 2024

Thanks for the quick reviews!

Well, I am not sure I would call 2 days a quick review. One of my goals over the next few months is to grow the reviewer capacity in arrow-rs .

@a10y a10y deleted the aduffy/cast-utf8-timestamp branch July 20, 2024 15:34
This pull request was closed.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support casting to/from Utf8View
3 participants