ARROW-15590: [C++] Add support for joins to the Substrait consumer #13078

vibhatha · 2022-05-06T06:38:00Z

Initial Version of Substrait Join Support

This PR doesn't support the complete join functionality, but it include the following features.
This will be a followed by a set of PRs to solve the remaining features [1].

Features included

Only Support Inner Join (A follow up PR would include the support for other join types)
Support Join operations with a single call-expression of types "equal" and "is_not_distinct_from"
Test cases to check the basic functionality and limitations

Todo:

Fix the Windows CI Issue

[1]. https://issues.apache.org/jira/browse/ARROW-16485

github-actions · 2022-05-06T06:38:18Z

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

github-actions · 2022-05-06T06:38:19Z

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

vibhatha · 2022-05-06T06:38:55Z

@westonpace adding an initial version of the PR. Please take a look.

vibhatha · 2022-05-10T15:28:05Z

@westonpace not sure why appveyor CI is failing...

westonpace

This looks like a good start for join support. I wonder how we might start building up some end-to-end tests that use Substrait. Any ideas?

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

cpp/src/arrow/compute/exec/options.h

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

vibhatha · 2022-05-12T07:43:15Z

This looks like a good start for join support. I wonder how we might start building up some end-to-end tests that use Substrait. Any ideas?

I had this in my mind. The neatest/fastest way is to use Substrait-Python integration. Or we need to write a data module in the C++ test suite to provide end-to-end testing. I used existing datasets from the Parquet test data submodule.
But Python may not have everything exposed, may be we can write one here.

I see most of the test cases in Substrait features are not end-to-end tests. May be we can start from now. WDYT?

westonpace · 2022-05-19T14:24:25Z

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

+          return Status::NotImplemented("Outer join type is not supported");
+        case 3:
+          return Status::NotImplemented("Left join type is not supported");
+        case 4:
+          return Status::NotImplemented("Right join type is not supported");
+        case 5:
+          return Status::NotImplemented("Semi join type is not supported");
+        case 6:
+          return Status::NotImplemented("Anti join type is not supported");


What is stopping us from supporting these joins? Is it just a matter of mapping to the appropriate Arrow equivalent enum? Or a matter of testing? Is there a follow-up PR?

Mainly, I am not 100% clear on mapping the definitions Substrait-Arrow. I thought about doing it in the follow up PR. Not any special reason stopping doing it for this PR.

Substrait Arrow

JOIN_TYPE_INNER INNER

JOIN_TYPE_OUTER FULL_OUTER

JOIN_TYPE_LEFT LEFT_OUTER

JOIN_TYPE_RIGHT RIGHT_OUTER

JOIN_TYPE_SEMI LEFT_SEMI

JOIN_TYPE_ANTI LEFT_ANTI

JOIN_TYPE_SINGLE -

- RIGHT_SEMI

- RIGHT_ANTI

JOIN_TYPE_SINGLE is interesting and I'd almost interpret that as an optional bool that could be added to any existing join. I'll create an issue to seek clarification. Let's add support for the rest in this PR since it should be straightforward enough.

Substrait doesn't have RIGHT_SEMI or RIGHT_ANTI and both of these can easily be created by just swapping the inputs so I'm not sure it is critical right now. Although it might help (for performance reasons) to keep the probe/build side separate from the side we want to keep the output for so they may come in handy in the future. I'm honestly not entirely sure. Either way, let's not worry about those for this PR.

substrait-io/substrait#206 for the JOIN_TYPE_SINGLE question but let's not block this PR waiting for that to resolve.

@westonpace thank you for the clarification. I will update the PR.

@westonpace updated the PR.

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

westonpace · 2022-05-19T15:18:06Z

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

+            "Only Support `equal` or `is_not_distinct_from` for join key comparison");
+      }
+
+      // TODO: Add Suffix support for Substrait


Create a follow-up JIRA or link to a Substrait issue.

Follow up JIRA created: https://issues.apache.org/jira/browse/ARROW-16624

Can you add the JIRA number in the comment?

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

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

vibhatha · 2022-05-25T00:30:18Z

@westonpace These two JIRAs [1] [2] will be helpful in finalizing the overall tasks associated with Substrait-Joins

[1] https://issues.apache.org/jira/browse/ARROW-16485
[2] https://issues.apache.org/jira/browse/ARROW-16624

Any other things that we should take care in this PR?

westonpace

Just a few minor nits

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

westonpace · 2022-05-25T09:06:11Z

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

+            "Only Support `equal` or `is_not_distinct_from` for join key comparison");
+      }
+
+      // TODO: Add Suffix support for Substrait


Can you add the JIRA number in the comment?

adding join type enum Co-authored-by: Weston Pace <weston.pace@gmail.com>

Co-authored-by: Weston Pace <weston.pace@gmail.com>

vibhatha · 2022-05-30T12:39:31Z

cc @westonpace not sure why this is occurring: https://github.com/apache/arrow/runs/6650523294?check_suite_focus=true#step:6:5132

westonpace

The CI failures appear to be unrelated.

github-actions bot added the Component: C++ label May 6, 2022

westonpace self-requested a review May 10, 2022 14:54

westonpace requested changes May 12, 2022

View reviewed changes

vibhatha force-pushed the arrow-15590 branch from 2cf56be to f944c7a Compare May 17, 2022 03:11

vibhatha requested a review from westonpace May 17, 2022 03:11

westonpace requested changes May 19, 2022

View reviewed changes

vibhatha requested a review from westonpace May 20, 2022 15:17

vibhatha added 10 commits May 23, 2022 18:18

temp update

726e5da

initial temp commit on value extraction

285c7e1

wip testing join

43bdeff

initial test bed added with ready draft PR

ec63532

adding unsupported join type status message

5c2a906

adding missing move statements

06c81dd

removing unnecessary comment

aa269a7

fenum check updated

c720a4a

adding hashjoin node options

bd35cc0

addressing PR reviews

514a45d

vibhatha force-pushed the arrow-15590 branch from ff5d281 to 514a45d Compare May 23, 2022 12:53

vibhatha added 2 commits May 23, 2022 19:25

fix format issue

8ebb4d5

adding other join types

e0b14a6

westonpace requested changes May 25, 2022

View reviewed changes

vibhatha and others added 3 commits May 30, 2022 12:54

Update cpp/src/arrow/engine/substrait/relation_internal.cc

e054bd6

adding join type enum Co-authored-by: Weston Pace <weston.pace@gmail.com>

Update cpp/src/arrow/engine/substrait/relation_internal.cc

f9028d6

Co-authored-by: Weston Pace <weston.pace@gmail.com>

Update cpp/src/arrow/engine/substrait/relation_internal.cc

e9dd05c

Co-authored-by: Weston Pace <weston.pace@gmail.com>

vibhatha and others added 5 commits May 30, 2022 12:55

Update cpp/src/arrow/engine/substrait/relation_internal.cc

64fd3d6

Co-authored-by: Weston Pace <weston.pace@gmail.com>

Update cpp/src/arrow/engine/substrait/relation_internal.cc

b43d504

Co-authored-by: Weston Pace <weston.pace@gmail.com>

Update cpp/src/arrow/engine/substrait/relation_internal.cc

d729a64

Co-authored-by: Weston Pace <weston.pace@gmail.com>

Update cpp/src/arrow/engine/substrait/relation_internal.cc

3484d51

Co-authored-by: Weston Pace <weston.pace@gmail.com>

adding jira id

6277ac5

vibhatha requested a review from westonpace May 30, 2022 07:43

westonpace approved these changes Jun 1, 2022

View reviewed changes

westonpace merged commit 2eb2573 into apache:master Jun 1, 2022

vibhatha mentioned this pull request Jun 2, 2022

Adding Join Suffix substrait-io/substrait#220

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARROW-15590: [C++] Add support for joins to the Substrait consumer #13078

ARROW-15590: [C++] Add support for joins to the Substrait consumer #13078

vibhatha commented May 6, 2022 •

edited

Loading

github-actions bot commented May 6, 2022

github-actions bot commented May 6, 2022

vibhatha commented May 6, 2022

vibhatha commented May 10, 2022

westonpace left a comment

vibhatha commented May 12, 2022

westonpace May 19, 2022

vibhatha May 19, 2022

westonpace May 23, 2022

westonpace May 23, 2022

vibhatha May 24, 2022

vibhatha May 25, 2022

westonpace May 19, 2022

vibhatha May 20, 2022

westonpace May 25, 2022

vibhatha commented May 25, 2022

westonpace left a comment

westonpace May 25, 2022

vibhatha commented May 30, 2022

westonpace left a comment

Substrait	Arrow
JOIN_TYPE_INNER	INNER
JOIN_TYPE_OUTER	FULL_OUTER
JOIN_TYPE_LEFT	LEFT_OUTER
JOIN_TYPE_RIGHT	RIGHT_OUTER
JOIN_TYPE_SEMI	LEFT_SEMI
JOIN_TYPE_ANTI	LEFT_ANTI
JOIN_TYPE_SINGLE	-
-	RIGHT_SEMI
-	RIGHT_ANTI

ARROW-15590: [C++] Add support for joins to the Substrait consumer #13078

ARROW-15590: [C++] Add support for joins to the Substrait consumer #13078

Conversation

vibhatha commented May 6, 2022 • edited Loading

github-actions bot commented May 6, 2022

github-actions bot commented May 6, 2022

vibhatha commented May 6, 2022

vibhatha commented May 10, 2022

westonpace left a comment

Choose a reason for hiding this comment

vibhatha commented May 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vibhatha commented May 25, 2022

westonpace left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vibhatha commented May 30, 2022

westonpace left a comment

Choose a reason for hiding this comment

vibhatha commented May 6, 2022 •

edited

Loading