[R] Allow Parquet and Arrow Dataset to be optional components

[asfimport]

Parquet requires thrift and it seems that thrift (at least as of version 0.12) does not compile on Solaris:

/export/home/X1svPYR/Rtemp/RtmptF1MlN/file75097d284891/thrift_ep-prefix/src/thrift_ep/lib/cpp/src/thrift/transport/THttpServer.cpp:� In member function virtual void apache::thrift::transport::THttpServer::parseHeader(char*):
/export/home/X1svPYR/Rtemp/RtmptF1MlN/file75097d284891/thrift_ep-prefix/src/thrift_ep/lib/cpp/src/thrift/transport/THttpServer.cpp:50:74:� error: �strcasestr was not declared in this scope
   #define THRIFT_strcasestr(haystack, needle) strcasestr(haystack, needle)
                                                                          ^�
/export/home/X1svPYR/Rtemp/RtmptF1MlN/file75097d284891/thrift_ep-prefix/src/thrift_ep/lib/cpp/src/thrift/transport/THttpServer.cpp:62:9:� �note: �in expansion of macro THRIFT_strcasestr�
     if (THRIFT_strcasestr(value, "chunked") != NULL) {

(along with some boost endian header deprecation warnings)

We could debug/patch that, or we could also make Parquet an optional feature in the R bindings. That might have some value anyway so that one could build a lighter/minimal R package, if that were helpful.

Reporter: Neal Richardson / @nealrichardson
Assignee: Ian Cook / @ianmcook

Related issues:

• [C++] Declare required Libs.private in arrow.pc package config (relates to)
• [R] Use roxygen @examplesIf tag in R docs (is related to)
• [R][Packaging] Dataset, Parquet off in autobrew and CRAN Mac builds (is depended upon by)
• [R][Packaging] Apply changes in r/tools/autobrew upstream (is depended upon by)

PRs and other links:

• GitHub Pull Request #9610

_{Note: This issue was originally created as ARROW-11735. Please see the migration documentation for further details.}

[asfimport]

Neal Richardson / @nealrichardson:
See r/data-raw/codegen.R for how this comes together (docs in comments at the top of the file), and look at the bottom of r/src/filesystem.cpp for an example of how this is working for s3. Changes to make:

• Make sure that everything that touches things in the parquet C++ namespace (C++ classes that are parquet::something, header #include statements of parquet paths) are inside #if defined(ARROW_R_WITH_PARQUET)
• Change the // [[arrow::export]] annotations on those functions to be // [[parquet::export]]
• Add "parquet" to the features list at the top of data-raw/codegen.R
• This is the tricky part: in configure and configure.win, do a check similar to how we look for whether ARROW_S3 was enabled in the C++ build, and if so add -DARROW_R_WITH_PARQUET to PKG_CFLAGS. This will be trickier on Windows because I don't think the rwinlib bundle includes ArrowOptions.cmake (but it will always have Parquet enabled). An alternative (perhaps more robust approach) would be to follow the TEST_CMD approach and test for parquet/s3 headers (current L179 of configure).
• In configure.win, you may also need to massage the PKG_LIBS since it has hard-coded that parquet and thrift are present (as they are in the rwinlib bundles). This is perhaps a separate issue from this (since this issue is particularly about Solaris and that's about doing different C++ dev builds on Windows.
• Once you've done this, the build should succeed without Parquet, but tests that involve Parquet will error. Revise skip_if_not_available() in r/tests/testthat/helper-skip.R to check arrow_with_parquet() if feature == "parquet" (this function is generated by codegen.R), then add skip_if_not_available("parquet") to all relevant tests.
• Search for arrow_with_s3() in the code and see where else we should do the same with arrow_with_parquet(). Among the uses:
• Add parquet to arrow_info() alongside where arrow_with_s3() is checked
• Wrap any parquet doc examples if (arrow_with_parquet())

[asfimport]

Ian Cook / @ianmcook:
One problem here: pkg-config reports that the Arrow Dataset module depends on Parquet

$ pkg-config --libs arrow-dataset
-L/usr/local/lib -larrow_dataset -lparquet -larrow 

Does it actually? From CMakeLists.txt, it doesn't look like it:

https://github.com/apache/arrow/blob/master/cpp/CMakeLists.txt#L339-L342

But there is a find_package(Parquet) call in FindArrowDataset.cmake:

https://github.com/apache/arrow/blob/master/cpp/cmake_modules/FindArrowDataset.cmake#L47

arrow-dataset.pc.in has parquet in the Requires::
https://github.com/apache/arrow/blob/master/cpp/src/arrow/dataset/arrow-dataset.pc.in#L24

c_glib/configure.ac puts -lparquet in ARROW_DATASET_LIBS:

https://github.com/apache/arrow/blob/master/c_glib/configure.ac#L225

In the dataset subcomponent CMakeLists.txt, everything Parquet-specific is guarded with if(ARROW_PARQUET):

https://github.com/apache/arrow/blob/master/cpp/src/arrow/dataset/CMakeLists.txt

[asfimport]

Neal Richardson / @nealrichardson:
This may only be for historical reasons: when libarrow_dataset was created, it only worked for Parquet files, so the hard dependency made sense.

I don't know how disruptive it would be to make that dependency conditional (cc @bkietz). If we didn't want to go that route, we could extend this work in the R package to make both the dataset and parquet features conditional--dataset would work the same way I described above, it's in its own namespace like parquet is, etc.

[asfimport]

Neal Richardson / @nealrichardson:
Issue resolved by pull request 9610
#9610