Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GpuIf and GpuCoalesce support array and struct types #2839

Merged
merged 15 commits into from
Jul 23, 2021
Merged

GpuIf and GpuCoalesce support array and struct types #2839

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
f93ff26
CaseWhen supports scalar of list&struct
firestarman Jun 28, 2021
47d24a0
GpuIf and GpuCoalesce support scalar of list&struct
firestarman Jun 29, 2021
e980afe
Address comments
firestarman Jun 30, 2021
846d2be
Merge branch 'branch-21.08' into casewhen-list-scalar
firestarman Jul 14, 2021
4f2631e
Support nested array and struct
firestarman Jul 14, 2021
ef5d0ed
Merge branch 'branch-21.08' into gpu-if-coalesce
firestarman Jul 16, 2021
182488c
Support nested list and struct
firestarman Jul 16, 2021
19a0642
tests update
firestarman Jul 16, 2021
0e27b0c
comment update
firestarman Jul 16, 2021
82e70f8
Merge branch 'branch-21.08' into casewhen-list-scalar
firestarman Jul 21, 2021
3ec8987
update the function declaration
firestarman Jul 21, 2021
a395e4a
Merge branch 'casewhen-list-scalar' into gpu-if-coalesce
firestarman Jul 21, 2021
09ea391
Merge branch 'branch-21.08' into gpu-if-coalesce
firestarman Jul 21, 2021
02f8399
Address the comments
firestarman Jul 22, 2021
5321ae7
correct the test name
firestarman Jul 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions docs/supported_ops.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3391,9 +3391,9 @@ Accelerator support is described below.
<td>S</td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><em>PS* (missing nested BINARY, CALENDAR, ARRAY, MAP, STRUCT, UDT)</em></td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><em>PS* (missing nested BINARY, CALENDAR, ARRAY, MAP, STRUCT, UDT)</em></td>
<td><b>NS</b></td>
</tr>
<tr>
Expand All @@ -3412,9 +3412,9 @@ Accelerator support is described below.
<td>S</td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><em>PS* (missing nested BINARY, CALENDAR, ARRAY, MAP, STRUCT, UDT)</em></td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><em>PS* (missing nested BINARY, CALENDAR, ARRAY, MAP, STRUCT, UDT)</em></td>
<td><b>NS</b></td>
</tr>
<tr>
Expand Down Expand Up @@ -7594,7 +7594,7 @@ Accelerator support is described below.
<td rowSpan="8">None</td>
<td rowSpan="4">project</td>
<td>predicate</td>
<td><em>PS (literal values are not supported)</em></td>
<td>S</td>
<td> </td>
<td> </td>
<td> </td>
Expand Down Expand Up @@ -7629,9 +7629,9 @@ Accelerator support is described below.
<td>S</td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><em>PS* (missing nested BINARY, CALENDAR, ARRAY, MAP, STRUCT, UDT)</em></td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><em>PS* (missing nested BINARY, CALENDAR, ARRAY, MAP, STRUCT, UDT)</em></td>
<td><b>NS</b></td>
</tr>
<tr>
Expand All @@ -7650,9 +7650,9 @@ Accelerator support is described below.
<td>S</td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><em>PS* (missing nested BINARY, CALENDAR, ARRAY, MAP, STRUCT, UDT)</em></td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><em>PS* (missing nested BINARY, CALENDAR, ARRAY, MAP, STRUCT, UDT)</em></td>
<td><b>NS</b></td>
</tr>
<tr>
Expand All @@ -7671,9 +7671,9 @@ Accelerator support is described below.
<td>S</td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><em>PS* (missing nested BINARY, CALENDAR, ARRAY, MAP, STRUCT, UDT)</em></td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><b>NS</b></td>
<td><em>PS* (missing nested BINARY, CALENDAR, ARRAY, MAP, STRUCT, UDT)</em></td>
<td><b>NS</b></td>
</tr>
<tr>
Expand Down
4 changes: 2 additions & 2 deletions integration_tests/src/main/python/conditionals_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

all_gens = all_gen + [NullGen()]

@pytest.mark.parametrize('data_gen', all_gens, ids=idfn)
@pytest.mark.parametrize('data_gen', all_gens + all_single_nested_gens, ids=idfn)
def test_if_else(data_gen):
(s1, s2) = gen_scalars_for_sql(data_gen, 2, force_no_nulls=not isinstance(data_gen, NullGen))
null_lit = get_null_lit_string(data_gen.data_type)
Expand Down Expand Up @@ -92,7 +92,7 @@ def test_nvl(data_gen):
'nvl(a, {})'.format(null_lit)))

#nvl is translated into a 2 param version of coalesce
@pytest.mark.parametrize('data_gen', all_gens, ids=idfn)
@pytest.mark.parametrize('data_gen', all_gens + all_single_nested_gens, ids=idfn)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is all_single_nested_gens defined?
can we add struct_gens_sample for deeper nesting?
we are also enabling arrays¸ can we add their gens too?

Copy link
Collaborator Author

@firestarman firestarman Jun 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is defined in PR #2826, on which this PR depends.
I will add deeper nesting if it is supported by PR rapidsai/cudf#8588 . But verification is required at first.

def test_coalesce(data_gen):
num_cols = 20
s1 = gen_scalar(data_gen, force_no_nulls=not isinstance(data_gen, NullGen))
Expand Down
24 changes: 16 additions & 8 deletions sql-plugin/src/main/scala/com/nvidia/spark/rapids/GpuOverrides.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1187,10 +1187,13 @@ object GpuOverrides {
expr[Coalesce] (
"Returns the first non-null argument if exists. Otherwise, null",
ExprChecks.projectNotLambda(
TypeSig.commonCudfTypes + TypeSig.NULL + TypeSig.DECIMAL, TypeSig.all,
(TypeSig.commonCudfTypes + TypeSig.NULL + TypeSig.DECIMAL).nested() +
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we can use _commonTypes for this common combination

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

TypeSig.ARRAY + TypeSig.STRUCT,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we going to add arbitrary nesting in a follow-up?

Copy link
Collaborator Author

@firestarman firestarman Jun 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will add deeper nesting if it is supported by PR rapidsai/cudf#8588 . But verification is required at first.

TypeSig.all,
repeatingParamCheck = Some(RepeatingParamCheck("param",
TypeSig.commonCudfTypes + TypeSig.NULL + TypeSig.DECIMAL,
TypeSig.all))),
(TypeSig.commonCudfTypes + TypeSig.NULL + TypeSig.DECIMAL).nested() +
TypeSig.ARRAY + TypeSig.STRUCT,
TypeSig.all))),
(a, conf, p, r) => new ExprMeta[Coalesce](a, conf, p, r) {
override def convertToGpu(): GpuExpression = GpuCoalesce(childExprs.map(_.convertToGpu()))
}),
Expand Down Expand Up @@ -1739,13 +1742,18 @@ object GpuOverrides {
}),
expr[If](
"IF expression",
ExprChecks.projectNotLambda(TypeSig.commonCudfTypes + TypeSig.NULL + TypeSig.DECIMAL,
ExprChecks.projectNotLambda(
(TypeSig.commonCudfTypes + TypeSig.NULL + TypeSig.DECIMAL).nested() +
TypeSig.ARRAY + TypeSig.STRUCT,
TypeSig.all,
Seq(ParamCheck("predicate", TypeSig.psNote(TypeEnum.BOOLEAN,
"literal values are not supported"), TypeSig.BOOLEAN),
ParamCheck("trueValue", TypeSig.commonCudfTypes + TypeSig.NULL + TypeSig.DECIMAL,
Seq(ParamCheck("predicate", TypeSig.BOOLEAN, TypeSig.BOOLEAN),
ParamCheck("trueValue",
(TypeSig.commonCudfTypes + TypeSig.NULL + TypeSig.DECIMAL).nested() +
TypeSig.ARRAY + TypeSig.STRUCT,
TypeSig.all),
ParamCheck("falseValue", TypeSig.commonCudfTypes + TypeSig.NULL + TypeSig.DECIMAL,
ParamCheck("falseValue",
(TypeSig.commonCudfTypes + TypeSig.NULL + TypeSig.DECIMAL).nested() +
TypeSig.ARRAY + TypeSig.STRUCT,
TypeSig.all))),
(a, conf, p, r) => new ExprMeta[If](a, conf, p, r) {
override def convertToGpu(): GpuExpression = {
Expand Down