ARROW-75: Fix handling of empty strings #32

danrobinson · 2016-03-22T15:16:36Z

Fixes ARROW-75 (and changes Python tests to verify that behavior).

wesm · 2016-03-22T15:45:01Z

cpp/src/arrow/types/primitive.h

@@ -168,7 +168,7 @@ class PrimitiveBuilder : public ArrayBuilder {
 int32_t new_capacity = util::next_power2(length_ + length);
 RETURN_NOT_OK(Resize(new_capacity));
 }
- memcpy(raw_buffer() + length_, values, length * elsize_);
+ if (length > 0) memcpy(raw_buffer() + length_, values, length * elsize_);


Put braces around this (with the memcpy on its own line)?

wesm · 2016-03-22T15:47:19Z

Could you also add a small unit test to types/string-test.cc (appending one or two empty strings and checking the resulting StringArray will suffice)?

danrobinson · 2016-03-22T16:45:12Z

It turns out the empty strings only cause a segfault when they are the first item in the array. Both the Python and C++ tests now test for this behavior.

wesm · 2016-03-22T17:00:34Z

You can add a pre-commit hook to run make lint to avoid cpplint-related travis failures (or also run make lint -- I forgot a lot until I added the pre-commit hook)

wesm · 2016-03-22T21:14:36Z

+1, thank you

wesm · 2016-03-22T21:16:49Z

Merged -- note you should be able to self-assign JIRAs now

…litate unit testing Several things in this patch * Adds PageReader abstraction, and a SerializedPageReader implementation according to the Parquet file format * Adds a MockPageReader and a couple unit tests demonstrating end-to-end test without creating a Parquet file * Adds a DataPageBuilder test fixture tool, may become part of the main write path later * Adds PlainEncoder implementation for a few primitive types * Fixes a few ColumnReader bugs exposed by the unit tests Author: Wes McKinney <wes@cloudera.com> Closes apache#32 from wesm/PARQUET-485 and squashes the following commits: aa33078 [Wes McKinney] Fix function doc e897a81 [Wes McKinney] Restore NumRequiredBits function after rebase ee4d97a [Wes McKinney] Change PageReader::NextPage API to return shared_ptr<Page>(nullptr) on eos 0324021 [Wes McKinney] Clarify some comments ec871c4 [Wes McKinney] Add include guards e63bbdd [Wes McKinney] Move vector_equal to util/test-common.h 44a78a1 [Wes McKinney] Refactor to decouple page deserialization from column reader so that mock data pages cna be constructed in unit tests.

@edani

Requires PARQUET-485 (apache#32) The boolean Encoding::PLAIN code path was using RleDecoder, inconsistent with other implementations of Parquet. This patch adds an implementation of plain encoding and uses BitReader instead of RleDecoder to decode plain-encoded boolean data. Unit tests to verify. Also closes PR apache#12. Thanks to @edani for reporting. Author: Wes McKinney <wes@cloudera.com> Closes apache#34 from wesm/PARQUET-454 and squashes the following commits: 01cb5a7 [Wes McKinney] Use a seed in the data generation 0bf5d8a [Wes McKinney] Fix inconsistencies with boolean PLAIN encoding.

…litate unit testing Several things in this patch * Adds PageReader abstraction, and a SerializedPageReader implementation according to the Parquet file format * Adds a MockPageReader and a couple unit tests demonstrating end-to-end test without creating a Parquet file * Adds a DataPageBuilder test fixture tool, may become part of the main write path later * Adds PlainEncoder implementation for a few primitive types * Fixes a few ColumnReader bugs exposed by the unit tests Author: Wes McKinney <wes@cloudera.com> Closes apache#32 from wesm/PARQUET-485 and squashes the following commits: aa33078 [Wes McKinney] Fix function doc e897a81 [Wes McKinney] Restore NumRequiredBits function after rebase ee4d97a [Wes McKinney] Change PageReader::NextPage API to return shared_ptr<Page>(nullptr) on eos 0324021 [Wes McKinney] Clarify some comments ec871c4 [Wes McKinney] Add include guards e63bbdd [Wes McKinney] Move vector_equal to util/test-common.h 44a78a1 [Wes McKinney] Refactor to decouple page deserialization from column reader so that mock data pages cna be constructed in unit tests. Change-Id: I7c4564cc7978d0cde430a7a2c6a2944ba01d80c8

@edani

Requires PARQUET-485 (apache#32) The boolean Encoding::PLAIN code path was using RleDecoder, inconsistent with other implementations of Parquet. This patch adds an implementation of plain encoding and uses BitReader instead of RleDecoder to decode plain-encoded boolean data. Unit tests to verify. Also closes PR apache#12. Thanks to @edani for reporting. Author: Wes McKinney <wes@cloudera.com> Closes apache#34 from wesm/PARQUET-454 and squashes the following commits: 01cb5a7 [Wes McKinney] Use a seed in the data generation 0bf5d8a [Wes McKinney] Fix inconsistencies with boolean PLAIN encoding. Change-Id: I1be5252c654d4864d14c3cdd70d63c507e0a9403

…litate unit testing Several things in this patch * Adds PageReader abstraction, and a SerializedPageReader implementation according to the Parquet file format * Adds a MockPageReader and a couple unit tests demonstrating end-to-end test without creating a Parquet file * Adds a DataPageBuilder test fixture tool, may become part of the main write path later * Adds PlainEncoder implementation for a few primitive types * Fixes a few ColumnReader bugs exposed by the unit tests Author: Wes McKinney <wes@cloudera.com> Closes apache#32 from wesm/PARQUET-485 and squashes the following commits: aa33078 [Wes McKinney] Fix function doc e897a81 [Wes McKinney] Restore NumRequiredBits function after rebase ee4d97a [Wes McKinney] Change PageReader::NextPage API to return shared_ptr<Page>(nullptr) on eos 0324021 [Wes McKinney] Clarify some comments ec871c4 [Wes McKinney] Add include guards e63bbdd [Wes McKinney] Move vector_equal to util/test-common.h 44a78a1 [Wes McKinney] Refactor to decouple page deserialization from column reader so that mock data pages cna be constructed in unit tests. Change-Id: I7c4564cc7978d0cde430a7a2c6a2944ba01d80c8

@edani

Requires PARQUET-485 (apache#32) The boolean Encoding::PLAIN code path was using RleDecoder, inconsistent with other implementations of Parquet. This patch adds an implementation of plain encoding and uses BitReader instead of RleDecoder to decode plain-encoded boolean data. Unit tests to verify. Also closes PR apache#12. Thanks to @edani for reporting. Author: Wes McKinney <wes@cloudera.com> Closes apache#34 from wesm/PARQUET-454 and squashes the following commits: 01cb5a7 [Wes McKinney] Use a seed in the data generation 0bf5d8a [Wes McKinney] Fix inconsistencies with boolean PLAIN encoding. Change-Id: I1be5252c654d4864d14c3cdd70d63c507e0a9403

…litate unit testing Several things in this patch * Adds PageReader abstraction, and a SerializedPageReader implementation according to the Parquet file format * Adds a MockPageReader and a couple unit tests demonstrating end-to-end test without creating a Parquet file * Adds a DataPageBuilder test fixture tool, may become part of the main write path later * Adds PlainEncoder implementation for a few primitive types * Fixes a few ColumnReader bugs exposed by the unit tests Author: Wes McKinney <wes@cloudera.com> Closes apache#32 from wesm/PARQUET-485 and squashes the following commits: aa33078 [Wes McKinney] Fix function doc e897a81 [Wes McKinney] Restore NumRequiredBits function after rebase ee4d97a [Wes McKinney] Change PageReader::NextPage API to return shared_ptr<Page>(nullptr) on eos 0324021 [Wes McKinney] Clarify some comments ec871c4 [Wes McKinney] Add include guards e63bbdd [Wes McKinney] Move vector_equal to util/test-common.h 44a78a1 [Wes McKinney] Refactor to decouple page deserialization from column reader so that mock data pages cna be constructed in unit tests. Change-Id: I7c4564cc7978d0cde430a7a2c6a2944ba01d80c8

@edani

Requires PARQUET-485 (apache#32) The boolean Encoding::PLAIN code path was using RleDecoder, inconsistent with other implementations of Parquet. This patch adds an implementation of plain encoding and uses BitReader instead of RleDecoder to decode plain-encoded boolean data. Unit tests to verify. Also closes PR apache#12. Thanks to @edani for reporting. Author: Wes McKinney <wes@cloudera.com> Closes apache#34 from wesm/PARQUET-454 and squashes the following commits: 01cb5a7 [Wes McKinney] Use a seed in the data generation 0bf5d8a [Wes McKinney] Fix inconsistencies with boolean PLAIN encoding. Change-Id: I1be5252c654d4864d14c3cdd70d63c507e0a9403

…litate unit testing Several things in this patch * Adds PageReader abstraction, and a SerializedPageReader implementation according to the Parquet file format * Adds a MockPageReader and a couple unit tests demonstrating end-to-end test without creating a Parquet file * Adds a DataPageBuilder test fixture tool, may become part of the main write path later * Adds PlainEncoder implementation for a few primitive types * Fixes a few ColumnReader bugs exposed by the unit tests Author: Wes McKinney <wes@cloudera.com> Closes apache#32 from wesm/PARQUET-485 and squashes the following commits: aa33078 [Wes McKinney] Fix function doc e897a81 [Wes McKinney] Restore NumRequiredBits function after rebase ee4d97a [Wes McKinney] Change PageReader::NextPage API to return shared_ptr<Page>(nullptr) on eos 0324021 [Wes McKinney] Clarify some comments ec871c4 [Wes McKinney] Add include guards e63bbdd [Wes McKinney] Move vector_equal to util/test-common.h 44a78a1 [Wes McKinney] Refactor to decouple page deserialization from column reader so that mock data pages cna be constructed in unit tests. Change-Id: I7c4564cc7978d0cde430a7a2c6a2944ba01d80c8

@edani

Requires PARQUET-485 (apache#32) The boolean Encoding::PLAIN code path was using RleDecoder, inconsistent with other implementations of Parquet. This patch adds an implementation of plain encoding and uses BitReader instead of RleDecoder to decode plain-encoded boolean data. Unit tests to verify. Also closes PR apache#12. Thanks to @edani for reporting. Author: Wes McKinney <wes@cloudera.com> Closes apache#34 from wesm/PARQUET-454 and squashes the following commits: 01cb5a7 [Wes McKinney] Use a seed in the data generation 0bf5d8a [Wes McKinney] Fix inconsistencies with boolean PLAIN encoding. Change-Id: I1be5252c654d4864d14c3cdd70d63c507e0a9403

This PR enables tests for `ARROW_COMPUTE`, `ARROW_DATASET`, `ARROW_FILESYSTEM`, `ARROW_HDFS`, `ARROW_ORC`, and `ARROW_IPC` (default on). #7131 enabled a minimal set of tests as a starting point. I confirmed that these tests pass locally with the current master. In the current TravisCI environment, we cannot see this result due to a lot of error messages in `arrow-utility-test`. ``` $ git log | head -1 commit ed5f534 % ctest ... Start 1: arrow-array-test 1/51 Test #1: arrow-array-test ..................... Passed 4.62 sec Start 2: arrow-buffer-test 2/51 Test #2: arrow-buffer-test .................... Passed 0.14 sec Start 3: arrow-extension-type-test 3/51 Test #3: arrow-extension-type-test ............ Passed 0.12 sec Start 4: arrow-misc-test 4/51 Test #4: arrow-misc-test ...................... Passed 0.14 sec Start 5: arrow-public-api-test 5/51 Test #5: arrow-public-api-test ................ Passed 0.12 sec Start 6: arrow-scalar-test 6/51 Test #6: arrow-scalar-test .................... Passed 0.13 sec Start 7: arrow-type-test 7/51 Test #7: arrow-type-test ...................... Passed 0.14 sec Start 8: arrow-table-test 8/51 Test #8: arrow-table-test ..................... Passed 0.13 sec Start 9: arrow-tensor-test 9/51 Test #9: arrow-tensor-test .................... Passed 0.13 sec Start 10: arrow-sparse-tensor-test 10/51 Test #10: arrow-sparse-tensor-test ............. Passed 0.16 sec Start 11: arrow-stl-test 11/51 Test #11: arrow-stl-test ....................... Passed 0.12 sec Start 12: arrow-concatenate-test 12/51 Test #12: arrow-concatenate-test ............... Passed 0.53 sec Start 13: arrow-diff-test 13/51 Test #13: arrow-diff-test ...................... Passed 1.45 sec Start 14: arrow-c-bridge-test 14/51 Test #14: arrow-c-bridge-test .................. Passed 0.18 sec Start 15: arrow-io-buffered-test 15/51 Test #15: arrow-io-buffered-test ............... Passed 0.20 sec Start 16: arrow-io-compressed-test 16/51 Test #16: arrow-io-compressed-test ............. Passed 3.48 sec Start 17: arrow-io-file-test 17/51 Test #17: arrow-io-file-test ................... Passed 0.74 sec Start 18: arrow-io-hdfs-test 18/51 Test #18: arrow-io-hdfs-test ................... Passed 0.12 sec Start 19: arrow-io-memory-test 19/51 Test #19: arrow-io-memory-test ................. Passed 2.77 sec Start 20: arrow-utility-test 20/51 Test #20: arrow-utility-test ...................***Failed 5.65 sec Start 21: arrow-threading-utility-test 21/51 Test #21: arrow-threading-utility-test ......... Passed 1.34 sec Start 22: arrow-compute-compute-test 22/51 Test #22: arrow-compute-compute-test ........... Passed 0.13 sec Start 23: arrow-compute-boolean-test 23/51 Test #23: arrow-compute-boolean-test ........... Passed 0.15 sec Start 24: arrow-compute-cast-test 24/51 Test #24: arrow-compute-cast-test .............. Passed 0.22 sec Start 25: arrow-compute-hash-test 25/51 Test #25: arrow-compute-hash-test .............. Passed 2.61 sec Start 26: arrow-compute-isin-test 26/51 Test #26: arrow-compute-isin-test .............. Passed 0.81 sec Start 27: arrow-compute-match-test 27/51 Test #27: arrow-compute-match-test ............. Passed 0.40 sec Start 28: arrow-compute-sort-to-indices-test 28/51 Test #28: arrow-compute-sort-to-indices-test ... Passed 3.33 sec Start 29: arrow-compute-nth-to-indices-test 29/51 Test #29: arrow-compute-nth-to-indices-test .... Passed 1.51 sec Start 30: arrow-compute-util-internal-test 30/51 Test #30: arrow-compute-util-internal-test ..... Passed 0.13 sec Start 31: arrow-compute-add-test 31/51 Test #31: arrow-compute-add-test ............... Passed 0.12 sec Start 32: arrow-compute-aggregate-test 32/51 Test #32: arrow-compute-aggregate-test ......... Passed 14.70 sec Start 33: arrow-compute-compare-test 33/51 Test #33: arrow-compute-compare-test ........... Passed 7.96 sec Start 34: arrow-compute-take-test 34/51 Test #34: arrow-compute-take-test .............. Passed 4.80 sec Start 35: arrow-compute-filter-test 35/51 Test #35: arrow-compute-filter-test ............ Passed 8.23 sec Start 36: arrow-dataset-dataset-test 36/51 Test #36: arrow-dataset-dataset-test ........... Passed 0.25 sec Start 37: arrow-dataset-discovery-test 37/51 Test #37: arrow-dataset-discovery-test ......... Passed 0.13 sec Start 38: arrow-dataset-file-ipc-test 38/51 Test #38: arrow-dataset-file-ipc-test .......... Passed 0.21 sec Start 39: arrow-dataset-file-test 39/51 Test #39: arrow-dataset-file-test .............. Passed 0.12 sec Start 40: arrow-dataset-filter-test 40/51 Test #40: arrow-dataset-filter-test ............ Passed 0.16 sec Start 41: arrow-dataset-partition-test 41/51 Test #41: arrow-dataset-partition-test ......... Passed 0.13 sec Start 42: arrow-dataset-scanner-test 42/51 Test #42: arrow-dataset-scanner-test ........... Passed 0.20 sec Start 43: arrow-filesystem-test 43/51 Test #43: arrow-filesystem-test ................ Passed 1.62 sec Start 44: arrow-hdfs-test 44/51 Test #44: arrow-hdfs-test ...................... Passed 0.13 sec Start 45: arrow-feather-test 45/51 Test #45: arrow-feather-test ................... Passed 0.91 sec Start 46: arrow-ipc-read-write-test 46/51 Test #46: arrow-ipc-read-write-test ............ Passed 5.77 sec Start 47: arrow-ipc-json-simple-test 47/51 Test #47: arrow-ipc-json-simple-test ........... Passed 0.16 sec Start 48: arrow-ipc-json-test 48/51 Test #48: arrow-ipc-json-test .................. Passed 0.27 sec Start 49: arrow-json-integration-test 49/51 Test #49: arrow-json-integration-test .......... Passed 0.13 sec Start 50: arrow-json-test 50/51 Test #50: arrow-json-test ...................... Passed 0.26 sec Start 51: arrow-orc-adapter-test 51/51 Test #51: arrow-orc-adapter-test ............... Passed 1.92 sec 98% tests passed, 1 tests failed out of 51 Label Time Summary: arrow-tests = 27.38 sec (27 tests) arrow_compute = 45.11 sec (14 tests) arrow_dataset = 1.21 sec (7 tests) arrow_ipc = 6.20 sec (3 tests) unittest = 79.91 sec (51 tests) Total Test time (real) = 79.99 sec The following tests FAILED: 20 - arrow-utility-test (Failed) Errors while running CTest ``` Closes #7142 from kiszk/ARROW-8754 Authored-by: Kazuaki Ishizaki <ishizaki@jp.ibm.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>

…to-useEncryption Change ssl parameter to UseEncryption

ARROW-75: Fix handling of empty strings

1ef3b75

wesm reviewed Mar 22, 2016
View reviewed changes

danrobinson added 2 commits March 22, 2016 11:58

ARROW-75: Fixed braces

722df19

ARROW-75: Changed tests

9604a21

ARROW-75: remove whitespace

cb8e527

asfgit closed this in 093f9bd Mar 22, 2016

guyuqi mentioned this pull request Nov 22, 2018

ARROW-3849: [C++] Leverage Armv8 crc32 extension instructions to accelerate the hash computation for Arm64 #3010

Closed

brills mentioned this pull request Jun 4, 2019

importing pyarrow and Tensorflow crashes #4472

Closed

jayhomn-bitquill pushed a commit to Bit-Quill/arrow that referenced this pull request Aug 10, 2022

Merge pull request apache#32 from rafael-telles/change-ssl-parameter-…

55eeed9

…to-useEncryption Change ssl parameter to UseEncryption

paleolimbot mentioned this pull request Jan 28, 2023

[R] Crash on MacOS (x86) when running tests with homebrew apache-arrow also installed #33903

Closed

prniii mentioned this pull request Jan 12, 2024

[Python] macOS Segfault on Import, Both arm64 and x86_64 #37010

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARROW-75: Fix handling of empty strings #32

ARROW-75: Fix handling of empty strings #32

danrobinson commented Mar 22, 2016

wesm Mar 22, 2016

wesm commented Mar 22, 2016

danrobinson commented Mar 22, 2016

wesm commented Mar 22, 2016

wesm commented Mar 22, 2016

wesm commented Mar 22, 2016

ARROW-75: Fix handling of empty strings #32

ARROW-75: Fix handling of empty strings #32

Conversation

danrobinson commented Mar 22, 2016

wesm Mar 22, 2016

Choose a reason for hiding this comment

wesm commented Mar 22, 2016

danrobinson commented Mar 22, 2016

wesm commented Mar 22, 2016

wesm commented Mar 22, 2016

wesm commented Mar 22, 2016