ARROW-5901: [Rust] Add equals to json arrays.#4940
ARROW-5901: [Rust] Add equals to json arrays.#4940liurenjie1024 wants to merge 4 commits intoapache:masterfrom
Conversation
|
@sunchao @andygrove @nevi-me Please take a look when you get a chance. |
wesm
changed the title
nevi-me
left a comment
nevi-me
left a comment
There was a problem hiding this comment.
LGTM @liurenjie1024 , I have a few minor comments and questions
| } | ||
| } | ||
|
|
||
| impl ArrowNativeType for i64 { |
There was a problem hiding this comment.
Is it fine that we implement this on ArrowNativeType (for i{32|64} which can also be dates and times)? Should we support temporal arrays? I know with time, we don't really have a JSON format, but we could have JSON timestamps that we might benefit from comparing.
There was a problem hiding this comment.
Do you mean ArrowPrimitiveType? In fact I don't think so, because temporal arrays already benefits from this because their native type is i32|i64.
There was a problem hiding this comment.
Yes, what I was considering was how we'd compare dates in JSON, as they're normally in the string format of 2019-07-29T05:07:07.771Z. I think it's not an issue though, so you may disregard my comment :)
There was a problem hiding this comment.
The challenging part is that primitive array's value method returns T::Native. I think we can still compare temporal type if we serialize date time type to time stamp in json.
Codecov Report
@@ Coverage Diff @@
## master #4940 +/- ##
===========================================
- Coverage 87.46% 82.43% -5.04%
===========================================
Files 994 86 -908
Lines 140562 25068 -115494
Branches 1418 0 -1418
===========================================
- Hits 122943 20664 -102279
+ Misses 17257 4404 -12853
+ Partials 362 0 -362
Continue to review full report at Codecov.
|
|
@wesm I noticed that you changed the title, but I can't see difference. Could you tell me the different so that I will not make the mistake again? |
|
@liurenjie1024 he changed the JIRA number, from 5091 to 5901 |
|
@sunchao are you merging it, or should I? |
|
@nevi-me go ahead to merge it |
4 participants
Checks whether an arrow array equals to an json array. This is motivated when I'm developing integration tests of parquet arrow reader. I use protobuf to generate both parquet data and json data, read parquet data to arrow, compare it with json data to verify the correct ness.