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

object_store/delimited: Fix TrailingEscape condition #6265

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

Turbo87
Copy link
Contributor

@Turbo87 Turbo87 commented Aug 16, 2024

This seems like a copy-paste mistake since checking is_quote twice is probably wrong...

@github-actions github-actions bot added the object-store Object Store Interface label Aug 16, 2024
@@ -126,7 +126,7 @@ impl LineDelimiter {
fn finish(&mut self) -> Result<bool> {
if !self.remainder.is_empty() {
ensure!(!self.is_quote, UnterminatedStringSnafu);
ensure!(!self.is_quote, TrailingEscapeSnafu);
ensure!(!self.is_escape, TrailingEscapeSnafu);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this covered by a test? If not, could you add one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this covered by a test?

I assume not, since I didn't have to adjust any tests 😅

If not, could you add one?

unfortunately I don't think I'm familiar enough with the code to be able to write such a test :-/

This seems like a copy-paste mistake since checking `is_quote` twice is probably wrong...
@Turbo87
Copy link
Contributor Author

Turbo87 commented Sep 4, 2024

/cc @tustvold :)

@tustvold tustvold merged commit 6da4793 into apache:master Sep 6, 2024
14 checks passed
@Turbo87 Turbo87 deleted the fix-condition branch January 7, 2025 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
object-store Object Store Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants