Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

fix unescaped '"' in json writing #812

Merged
merged 2 commits into from
Feb 5, 2022

Conversation

ritchie46
Copy link
Collaborator

fixes #811.

This is a quick hack that also searches for " before branching.

However, the second branch first did an unnecessary heap convertion from &str -> String. I removed it and the tests still succeed.

I think that we maybe should not branch at all now we don' do this heap allocation? Because we are now doing a double scan over the data determine which path we must take.

@codecov
Copy link

codecov bot commented Feb 5, 2022

Codecov Report

Merging #812 (010b714) into main (9a8edd0) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #812      +/-   ##
==========================================
- Coverage   71.16%   71.15%   -0.01%     
==========================================
  Files         327      327              
  Lines       17515    17508       -7     
==========================================
- Hits        12464    12458       -6     
+ Misses       5051     5050       -1     
Impacted Files Coverage Δ
src/io/json/write/serialize.rs 88.28% <100.00%> (+0.15%) ⬆️

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 9a8edd0...010b714. Read the comment docs.

@jorgecarleitao
Copy link
Owner

Thanks! I am working on a alloc free version, one sec :P

@jorgecarleitao
Copy link
Owner

Alternative: #813

@jorgecarleitao
Copy link
Owner

Actually, I think your proposal is better: do not branch and always use serde_json 👍

@ritchie46
Copy link
Collaborator Author

Haha.. just let me know which PR that will be. 🙈

@jorgecarleitao
Copy link
Owner

This one is way better. Thanks for the quick fix!

@jorgecarleitao jorgecarleitao added the bug Something isn't working label Feb 5, 2022
ritchie46 added a commit to ritchie46/arrow2 that referenced this pull request Feb 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unescaped " in value produces invalid json when encoded
2 participants