ARROW-519: [C++] Refactor array comparison code into a compare.h / compare.cc in part to resolve Xcode 6.1 linker issue#308
ARROW-519: [C++] Refactor array comparison code into a compare.h / compare.cc in part to resolve Xcode 6.1 linker issue#308wesm wants to merge 5 commits intoapache:masterfrom
Conversation
…n unit. Use visitor pattern. Also may resolve Xcode bug reported in ARROW-519 Change-Id: I288609704858f34f3e6a9fff442db9981144d862
Change-Id: Ia692b329342b36b9d1e38a7a33f74a5644dca921
Change-Id: I36d4c80c63239fdbc808a1637757c8f061a84fa8
wesm
changed the title
wesm
changed the title
…s of length-0 strings
…all empty strings Change-Id: I163de1772353fd47f9f07cd8a3e25d344cf6aa85
|
This turned out to be an Xcode 6.1 issue -- the build issue in https://github.com/conda-forge/arrow-cpp-feedstock was because the Travis CI image hadn't been updated. To take advantage of a short build queue on a Sunday night, I'm going to merge this as soon as I get a green build (I tested that parquet-cpp and pyarrow work with these changes). @xhochy if you have any comments or want to refactor this further, happy to help |
|
+1 |
| return true; | ||
| bool are_equal = false; | ||
| Status error = | ||
| ArrayRangeEquals(*this, *arr, start_idx, end_idx, other_start_idx, &are_equal); |
There was a problem hiding this comment.
This seems like a good candidate for a macro.
| set(CMAKE_CXX_FLAGS "${CXX_COMMON_FLAGS} ${CMAKE_CXX_FLAGS}") | ||
|
|
||
| # Enable perf and other tools to work properly | ||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer") |
There was a problem hiding this comment.
This can affect performance, I'd rather have a target ReleaseWithDebugInfo that has -fno-omit-frame-pointer -g set.
There was a problem hiding this comment.
See discussion on https://issues.cloudera.org/browse/IMPALA-4132 — if the performance penality is < 1%, then having it always on seems OK to me for easier perf debugging and core dumps. Kudu (also very performance sensitive) leaves this in too https://github.com/apache/kudu/blob/master/CMakeLists.txt#L176
I'm OK with removing it too
There was a problem hiding this comment.
Ok, that is very small. Leave it until we see that it changes significantly.
…ield cc @xhochy @itaiin @advancedxy We are not yet able to correctly to read structs. This showed up in ARROW-601, so this raises an exception for now Author: Wes McKinney <wes.mckinney@twosigma.com> Closes apache#308 from wesm/PARQUET-963 and squashes the following commits: da42932 [Wes McKinney] Return NotImplemented when attempting to read a struct field Change-Id: I57aada07f588689f98bb9b60d240f8cd7e249577
2 participants
This should also pave the way for more user-friendly reporting of "why are the arrays not equal" per ARROW-517