ARROW-15238: [C++] ARROW_ENGINE module with substrait consumer #11707

bkietz · 2021-11-15T18:27:47Z

Superseded by #12279

cpp/cmake_modules/DefineOptions.cmake

cpp/src/arrow/array/builder_base.h

cpp/src/arrow/engine/CMakeLists.txt

cpcloud · 2021-11-22T11:06:42Z

dev/archery/archery/lang/cpp.py

What's the motivation/effect of setting this to True? Always building static libs as well as shared?

I only changed this for consistency with the cmake defaults, which these otherwise follow

cpp/src/arrow/engine/substrait/type_internal.cc

cpp/src/arrow/engine/api.h

cpp/src/arrow/engine/substrait/extension_types.cc

cpp/src/arrow/engine/substrait/serde_test.cc

kszucs · 2021-12-13T20:27:18Z

cpp/src/arrow/CMakeLists.txt

ARROW_ENGINE depends on ARROW_COMPUTE, can we set it somewhere?

Perhaps consider to place it under the compute subdirectory?

I believe ARROW_ENGINE may also someday depend on datasets, ipc, parquet, and maybe even flight. For example, a substrait plan will generally start with a scan node (datasets) and the engine may need to use spillover (ipc / parquet) and we might want to send data to or receive data from flight nodes.

Some of this we could probably avoid using more indirection (e.g. substrait consumer defines a "table provider" and the user can use the "datasets table provider" to link the two modules) but to start with it might be easier to just do the simple thing.

Either way, I think we will eventually want a standalone engine module that isn't really just a child of compute so I'm kind of in favor of it being a peer (and not a child) of compute.

See: ARROW-15238 (which this PR satisfies)

ARROW_ENGINE does now depend on datasets, where a scan node is used to wrap substrait::ReadRels

github-actions · 2022-01-21T15:09:29Z

https://issues.apache.org/jira/browse/ARROW-15238

github-actions · 2022-01-21T15:09:30Z

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

jvanstraten · 2022-01-27T20:05:30Z

This can be closed, see #12279 for continuation.

@bkietz

Continuation of #11707. I'm taking over from @bkietz for now because he's unavailable right now for personal reasons. Closes #12279 from jvanstraten/substrait-consumer Lead-authored-by: Benjamin Kietzman <[email protected]> Co-authored-by: Jeroen van Straten <[email protected]> Co-authored-by: Weston Pace <[email protected]> Signed-off-by: Weston Pace <[email protected]>

@bkietz

Continuation of apache#11707. I'm taking over from @bkietz for now because he's unavailable right now for personal reasons. Closes apache#12279 from jvanstraten/substrait-consumer Lead-authored-by: Benjamin Kietzman <[email protected]> Co-authored-by: Jeroen van Straten <[email protected]> Co-authored-by: Weston Pace <[email protected]> Signed-off-by: Weston Pace <[email protected]>

@bkietz

* ARROW-15238: [C++] ARROW_ENGINE module with substrait consumer Continuation of apache#11707. I'm taking over from @bkietz for now because he's unavailable right now for personal reasons. Closes apache#12279 from jvanstraten/substrait-consumer Lead-authored-by: Benjamin Kietzman <[email protected]> Co-authored-by: Jeroen van Straten <[email protected]> Co-authored-by: Weston Pace <[email protected]> Signed-off-by: Weston Pace <[email protected]> * ARROW-15700: [C++] Compilation error on Ubuntu 18.04 Modified Substrait consumer interaction with libprotobuf to support versions down to 3.0.0, which is the minimum required due to Substrait's usage of proto3 syntax. Tested locally with: ``` export ARROW_PROTOBUF_URL=https://github.com/protocolbuffers/protobuf/releases/download/v3.0.0/protobuf-cpp-3.0.0.tar.gz cmake \ --preset ninja-debug \ -DProtobuf_SOURCE=BUNDLED \ -DARROW_PROTOBUF_BUILD_VERSION=v3.0.0 \ -DARROW_PROTOBUF_BUILD_SHA256_CHECKSUM=318e8f375fb4e5333975a40e0d1215e855b4a8c581d692eb0eb7df70db1a8d4e ``` (Is there an easier way to do this without modifying versions.txt or 751fb9d? Also, the env var is needed only because Google isn't at all consistent with their release file naming that far back.) It'd also be nice to add this to CI, but it's probably excessive to always run for a PR, unless combined with some other run. Closes apache#12448 from jvanstraten/ARROW-15700-Compilation-error-on-Ubuntu-18-04 Authored-by: Jeroen van Straten <[email protected]> Signed-off-by: Weston Pace <[email protected]> (cherry picked from commit 7d16a78) * ARROW-15258: [C++] Easy options to create a source node from a table This PR includes the addition of `TableSourceNode` to create a `ExecNode` easily using a table as the data source. ### TODO - [x] Fix test case for chunk_size Closes apache#12267 from vibhatha/arrow-15258-rb Authored-by: Vibhatha Abeykoon <[email protected]> Signed-off-by: Weston Pace <[email protected]> (cherry picked from commit fffdca2) * ARROW-15709: [C++] Compilation of ARROW_ENGINE fails if doing an "inline" build This should fix: - inline builds in general (ARROW-15709); - [weird stuff with inline builds causing non-tracked files to be deleted](https://github.com/apache/arrow;/pull/12444#issuecomment-1043143303) that the [previous fix](apache#12444) for the above [caused](apache#12454) - dependencies on git for downloading dependencies (ARROW-15760); - the build process for Substrait previously being treated as something too special to use Arrow's normal method for dealing with third-party dependencies (i.e. `ThirdpartyToolchain.cmake`) --- Initial attempt at making something functional to solve this issue properly. The use of `add_arrow_lib` in `ThirdpartyToolchain.cmake` is certainly odd, and I'm sure I'm not following best practices in that file in general. I could use some advice on what the proper way to do this would be. Some of the issues: - The CMake property specifying that a path refers to a generated file is scoped only to the current CMake file, so only moving the `externalproject_add` over to `ThirdpartyToolchain.cmake` resulted in the `add_arrow_lib` in `src/arrow/engine` failing due to missing source files. An object library didn't seem to resolve it either, and there are probably portability issues with that anyway, so that's why I ended up just using `add_arrow_lib`. - Unlike all the other third-party dependencies (AFAICT), Substrait can't currently be installed, so `Substrait_SOURCE=SYSTEM` makes no sense. In the end I just decided to override it, but that's probably not ideal. - Substrait doesn't have releases yet, so I had to resort to a git hash instead. </details> Closes apache#12457 from jvanstraten/ARROW-15709-Compilation-of-ARROW-ENGINE-fails-if-doi Authored-by: Jeroen van Straten <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]> (cherry picked from commit 89cc6b3) * ARROW-15830: [C++] Ensure target directory exists before running Substrait generation Closes apache#12548 from pitrou/ARROW-15830-ubuntu-cpp-bundled Authored-by: Antoine Pitrou <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]> (cherry picked from commit e989fb3) * ARROW-15850: [C++] Engine substrait headers missing from install Closes apache#12569 from westonpace/feature/ARROW-15850--substrait-headers-missing Authored-by: Weston Pace <[email protected]> Signed-off-by: David Li <[email protected]> (cherry picked from commit 8fce593) * Enable TPCH Q6 & Q1 Co-authored-by: Benjamin Kietzman <[email protected]> Co-authored-by: Jeroen van Straten <[email protected]> Co-authored-by: Weston Pace <[email protected]> Co-authored-by: Vibhatha Abeykoon <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]>

bkietz requested a review from cpcloud November 15, 2021 18:27

github-actions bot added Component: FlightRPC Component: C++ labels Nov 15, 2021

bkietz force-pushed the substrait-consumer branch from 241d7ac to 42067e7 Compare November 18, 2021 19:28

cpcloud reviewed Nov 22, 2021

View reviewed changes

cpcloud mentioned this pull request Nov 22, 2021

Types: Decimal literals should specify the meaning of their bytes substrait-io/substrait#84

Closed

bkietz force-pushed the substrait-consumer branch 2 times, most recently from f311208 to 392af8a Compare November 30, 2021 22:31

kszucs reviewed Dec 13, 2021

View reviewed changes

cpp/src/arrow/engine/api.h Outdated Show resolved Hide resolved

kszucs reviewed Dec 13, 2021

View reviewed changes

cpp/src/arrow/engine/substrait/extension_types.cc Outdated Show resolved Hide resolved

kszucs reviewed Dec 13, 2021

View reviewed changes

cpp/src/arrow/engine/substrait/serde_test.cc Outdated Show resolved Hide resolved

kszucs reviewed Dec 13, 2021

View reviewed changes

bkietz force-pushed the substrait-consumer branch 2 times, most recently from 26ce7a6 to 3650484 Compare January 14, 2022 19:38

bkietz marked this pull request as ready for review January 20, 2022 22:12

bkietz changed the title ~~[WIP] Substrait consumer~~ Substrait consumer Jan 20, 2022

bkietz added 14 commits January 20, 2022 20:34

first pass at using substrait protobufs

ff51e35

add conversion of types and a basic roundtrip test

906aacf

reorganize to engine/substrait/{proto}_internal.cc,h

5fc6c34

add basic literal conversions

151cf72

don't rely on Datum::type in DataEq matcher

1cf679b

add substrait_gen_verify, allow configurable substrait repo and tag

6dce17b

Expose NamedStruct<=>Schema serde

53c7c7d

add if_else <-> IfThen conversion

cd919f3

rebase, catch up to changes in substrait

c079359

finish catching up with substrait's new field references

5c69fb1

add SubstraitFromJSON

2a77d28

port more tests to JSON

a5c25de

add more DataEq matchers

873ec23

use Date64 for substrait::date

05be909

bkietz added 2 commits January 20, 2022 20:35

Add a test for extraction of an ExtensionSet from a Plan

d237377

add a roundtrip test for calling an extension function

5e0d6e5

bkietz force-pushed the substrait-consumer branch from c9f2058 to 5e0d6e5 Compare January 21, 2022 01:39

bkietz changed the title ~~Substrait consumer~~ ARROW-15238: [C++] ARROW_ENGINE module with substrait consumer Jan 21, 2022

apache deleted a comment from github-actions bot Jan 21, 2022

repair status_test::MatcherExplanations

cd22ef0

bkietz added 15 commits January 21, 2022 10:12

removing old generated extension files

1dc4c9d

unity: ensure globals are unique within a TU

ed5b0d5

ensure generated files are also excluded from lint_cpp_cli

5d61724

substrait consumer api cleanup

37b5673

ensure generated files are also excluded from rat

5b362f9

put actual json in engine_substrait_consumption.cc

b1a9bba

msvc: suppress C4251 (needs dll-interface)

b6499ae

remove duplicate ARROW_ENGINE option

1473dc2

ensure ARROW_DATASET is available if ARROW_ENGINE=on

107c5b7

try to localize suppressions, try defining LIBPROTOBUF_EXPORTS

5012be6

run cmake-format

2c73334

msvc: just suppress C4251, more int/size_t fixes

65c2ee8

msvc: one more int/size_t fix

6d8ebc7

msvc: one more int/size_t fix

adfe196

use libprotobuf 3.19

0a965c5

jvanstraten mentioned this pull request Jan 27, 2022

ARROW-15238: [C++] ARROW_ENGINE module with substrait consumer #12279

Closed

westonpace closed this Jan 27, 2022

asfimport mentioned this pull request Feb 17, 2022

[C++] Create "engine" module for the query engine #30735

Closed

ARROW-15238: [C++] ARROW_ENGINE module with substrait consumer #11707

ARROW-15238: [C++] ARROW_ENGINE module with substrait consumer #11707

Uh oh!

Conversation

bkietz commented Nov 15, 2021 • edited by ianmcook Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cpcloud Nov 22, 2021

Choose a reason for hiding this comment

Uh oh!

bkietz Jan 21, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kszucs Dec 13, 2021

Choose a reason for hiding this comment

Uh oh!

westonpace Jan 11, 2022

Choose a reason for hiding this comment

Uh oh!

bkietz Jan 21, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 21, 2022

Uh oh!

github-actions bot commented Jan 21, 2022

Uh oh!

jvanstraten commented Jan 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bkietz commented Nov 15, 2021 •

edited by ianmcook

Loading