ARROW-203: Python: Basic filename based Parquet read/write #83

xhochy · 2016-05-29T09:39:12Z

No description provided.

xhochy · 2016-05-29T09:48:00Z

Travis fails because we do not build parquet. This raises now the question how we should pull in parquet-cpp in the ci setup. Using a git-commit hash or just pull HEAD? Also via the thirdparty scripts or via a different approach?

xhochy · 2016-05-29T17:55:00Z

Some thoughts on my questions from above:

I would fix a commit-hash / release for parquet-cpp in the thirdparty-build
For now, we could use the existing thirdparty build infrastructure. There may be a need in future to have thridparty builds inside CMake for compiler option passing etc (e.g. for full debug build or when using LTO) but I would do that then separately en bloc.

emkornfield · 2016-05-29T23:05:23Z

python/pyarrow/parquet.pyx

+
+    # Must be in one expression to avoid calling std::move which is not possible
+    # in Cython (due to missing rvalue support)
+    reader = unique_ptr[FileReader](new FileReader(default_memory_pool(),


I think you can use reader.reset, so you aren't relying on the move functionality/R-Values references at all?

No, this is not due to the assignment to reader but about forwarding the return value of ParquetFileReader.OpenFile(filename).

We are probably going run into some issues with Cython's lackluster support for C++11 features. What I've done in other cases is create a shim layer either as a header-only or header + companion .cpp file that provides a simplified API for the Cython caller. I think for now it's good to get this end-to-end use case working

emkornfield · 2016-05-30T20:17:22Z

seems reasonable to me. I'm not super familiar with this part of the code yet, so getting some feedback from @wesm also likely makes sense. (Also it looks like the build is failing with the latest push)

wesm · 2016-06-01T08:00:31Z

cpp/src/arrow/parquet/parquet-io-test.cc

+  std::shared_ptr<Table> out;
+  ReadTableFromFile(std::move(file_reader), &out);
+  ASSERT_EQ(1, out->num_columns());
+  ASSERT_EQ(100, out->num_rows());


maybe good to get in the habit of doing const int num_rows = 100

wesm · 2016-06-01T08:31:55Z

it's really great that you got this working!

re: the thirdparty toolchain, at least for testing the Python side, we may be able to use the conda dev artifacts to simplify things (https://anaconda.org/apache/parquet-cpp). Having to build parquet-cpp from source (and its thirdparty, especially Thrift) kind of stinks. let me know what you think

wesm · 2016-06-09T22:46:39Z

cpp/src/arrow/parquet/writer.cc

+    if (!primitive_array) {
+      PARQUET_IGNORE_NOT_OK(writer.Close());
+      return Status::NotImplemented("Table must consist of PrimitiveArray instances");
+    }


@emkornfield per our related discussion about strings, at least in Parquet-land, variable and fixed-length binary values (and strings) are considered primitive types. So you could have a table of all string columns and it would be still semantically "flat". I don't think it's a big deal but it will impact code like this (obviously we will want to address nested data as soon as we can here)

@wesm Thats a fair point, but I'm not sure the semantic flatness makes a big difference here. The complicated part of converting the arrow binary/strings to parquet still remains and the best way to convert the data depends on the encoding. If it is plain encoding, then we would need to convert the ParquetByteArray, if its delta encoded length then a new batch API for the parquet writer is likely more appropriate.

I'm not attached to either representation so if we can get the type system to help better support some use-cases I'm for it. But it feels like for parquet the advantages of making string types primitive might not necessarily solve the harder problems. This debate will all go away when we JIT everything, right ;-) ?

It's more of a question of the type hierarchy. Conceivably we could have code involving std::is_base_of with PrimitiveArray. Does not need to be resolved here

wesm · 2016-06-09T22:56:17Z

I left minor comments but this is a great start to get this working. Can you add a JIRA for adding support and testing for strings, boolean, and smaller integer types (int8, 16). We will also need to deal with unsigned integers (but the will come back at signed integers until Parquet 2.0 happens someday...c'est la vie)

xhochy · 2016-06-10T13:59:37Z

Addressed all comments. For the conda things, I probably need some expert advice (toolchain and LD_LIBRARY_PATH) otherwise everything is included.

wesm · 2016-06-10T22:00:20Z

Thank you. In the interest of getting this in I'm merging this now; we should address the build/packaging comment separately (particularly if conda install pyarrow -c apache/channel/dev doesn't work out of the box after this patch)

wesm · 2016-06-10T22:00:28Z

+1

Introducing a cache to hold the projectors and filters for re-use. The cache is a LRU that can hold 100 entries.

…etFileReader Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#83 from majetideepak/PARQUET-559 and squashes the following commits: d7f9c47 [Deepak Majeti] modified ParquetFileReader API b5269f4 [Deepak Majeti] modified travis ci script to print log on failure 940de12 [Deepak Majeti] fixed memory leak 6de0b70 [Deepak Majeti] print logs e5ea341 [Deepak Majeti] print failure 9462954 [Deepak Majeti] try fixing test failure 3a0a1d7 [Deepak Majeti] modified External Stream 9d8b44c [Deepak Majeti] fixed formatting 11b3e6f [Deepak Majeti] Enable an external InputStream as a source to the ParquetFileReader

Introducing a cache to hold the projectors and filters for re-use. The cache is a LRU that can hold 100 entries.

…etFileReader Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#83 from majetideepak/PARQUET-559 and squashes the following commits: d7f9c47 [Deepak Majeti] modified ParquetFileReader API b5269f4 [Deepak Majeti] modified travis ci script to print log on failure 940de12 [Deepak Majeti] fixed memory leak 6de0b70 [Deepak Majeti] print logs e5ea341 [Deepak Majeti] print failure 9462954 [Deepak Majeti] try fixing test failure 3a0a1d7 [Deepak Majeti] modified External Stream 9d8b44c [Deepak Majeti] fixed formatting 11b3e6f [Deepak Majeti] Enable an external InputStream as a source to the ParquetFileReader Change-Id: I4064b32fad83c9dfe9801538fd3a1a949ae98366

Introducing a cache to hold the projectors and filters for re-use. The cache is a LRU that can hold 100 entries.

[oap-native-sql] Cast to String and cast to smaller scale type support

emkornfield reviewed May 29, 2016
View reviewed changes

wesm reviewed Jun 1, 2016
View reviewed changes

wesm reviewed Jun 9, 2016
View reviewed changes

xhochy added 5 commits June 10, 2016 11:56

Use unique_ptr and shared_ptr from Cython

000e1e3

Do not set LD_LIBRARY_PATH in python build

8d90d3f

Set LD_LIBRARY_PATH in python build

ec07768

Make code more readable by using using

38d786c

Remove FindParquet duplication

405f85d

asfgit closed this in ec66ddd Jun 10, 2016

wesm mentioned this pull request Jun 10, 2016

ARROW-210: Cleanup of the string related types in C++ code base #85

Closed

xhochy deleted the arrow-203 branch March 7, 2017 16:16

praveenbingo added a commit to praveenbingo/arrow that referenced this pull request Aug 30, 2018

GDV-31:[C++]Caching projectors and filters for re-use. (apache#83)

e07115b

Introducing a cache to hold the projectors and filters for re-use. The cache is a LRU that can hold 100 entries.

praveenbingo added a commit to praveenbingo/arrow that referenced this pull request Aug 30, 2018

GDV-31:[C++]Caching projectors and filters for re-use. (apache#83)

94ed2d6

Introducing a cache to hold the projectors and filters for re-use. The cache is a LRU that can hold 100 entries.

praveenbingo added a commit to praveenbingo/arrow that referenced this pull request Aug 30, 2018

GDV-31:[C++]Caching projectors and filters for re-use. (apache#83)

8656a7f

Introducing a cache to hold the projectors and filters for re-use. The cache is a LRU that can hold 100 entries.

praveenbingo added a commit to praveenbingo/arrow that referenced this pull request Aug 30, 2018

GDV-31:[C++]Caching projectors and filters for re-use. (apache#83)

739e084

Introducing a cache to hold the projectors and filters for re-use. The cache is a LRU that can hold 100 entries.

praveenbingo added a commit to praveenbingo/arrow that referenced this pull request Sep 4, 2018

GDV-31:[C++]Caching projectors and filters for re-use. (apache#83)

fd55d9f

Introducing a cache to hold the projectors and filters for re-use. The cache is a LRU that can hold 100 entries.

praveenbingo added a commit to praveenbingo/arrow that referenced this pull request Sep 10, 2018

GDV-31:[C++]Caching projectors and filters for re-use. (apache#83)

3c644f3

Introducing a cache to hold the projectors and filters for re-use. The cache is a LRU that can hold 100 entries.

praveenbingo added a commit to praveenbingo/arrow that referenced this pull request Sep 10, 2018

GDV-31:[C++]Caching projectors and filters for re-use. (apache#83)

3f5eb78

Introducing a cache to hold the projectors and filters for re-use. The cache is a LRU that can hold 100 entries.

xuechendi pushed a commit to xuechendi/arrow that referenced this pull request Aug 19, 2020

Merge pull request apache#83 from xuechendi/wip_cast

661ddbe

[oap-native-sql] Cast to String and cast to smaller scale type support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARROW-203: Python: Basic filename based Parquet read/write #83

ARROW-203: Python: Basic filename based Parquet read/write #83

xhochy commented May 29, 2016

xhochy commented May 29, 2016

xhochy commented May 29, 2016

emkornfield May 29, 2016 •

edited

Loading

xhochy May 30, 2016

wesm Jun 1, 2016

emkornfield commented May 30, 2016

wesm Jun 1, 2016

wesm commented Jun 1, 2016

wesm Jun 9, 2016

emkornfield Jun 10, 2016

wesm Jun 10, 2016

wesm commented Jun 9, 2016

xhochy commented Jun 10, 2016

wesm commented Jun 10, 2016

wesm commented Jun 10, 2016

ARROW-203: Python: Basic filename based Parquet read/write #83

ARROW-203: Python: Basic filename based Parquet read/write #83

Conversation

xhochy commented May 29, 2016

xhochy commented May 29, 2016

xhochy commented May 29, 2016

emkornfield May 29, 2016 • edited Loading

Choose a reason for hiding this comment

xhochy May 30, 2016

Choose a reason for hiding this comment

wesm Jun 1, 2016

Choose a reason for hiding this comment

emkornfield commented May 30, 2016

wesm Jun 1, 2016

Choose a reason for hiding this comment

wesm commented Jun 1, 2016

wesm Jun 9, 2016

Choose a reason for hiding this comment

emkornfield Jun 10, 2016

Choose a reason for hiding this comment

wesm Jun 10, 2016

Choose a reason for hiding this comment

wesm commented Jun 9, 2016

xhochy commented Jun 10, 2016

wesm commented Jun 10, 2016

wesm commented Jun 10, 2016

emkornfield May 29, 2016 •

edited

Loading