[SPARK-40809] [CONNECT] [FOLLOW] Support `alias()` in Python client #38631

grundprinzip · 2022-11-11T21:05:13Z

What changes were proposed in this pull request?

This extends the implementation of column aliases in Spark Connect with supporting lists of column names and providing the appropriate implementation for the Python side.

Why are the changes needed?

Compatibility

Does this PR introduce any user-facing change?

No

How was this patch tested?

UT in Python and Scala

grundprinzip · 2022-11-11T21:08:12Z

@amaliujia @cloud-fan @HyukjinKwon @hvanhovell

python/pyspark/sql/connect/column.py

connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala

python/pyspark/sql/connect/column.py

AmplabJenkins · 2022-11-13T03:45:56Z

Can one of the admins verify this patch?

amaliujia

LGTM

Per https://spark.apache.org/contributing.html we should follow the following when adding new tests:

def test_case(self):
    # SPARK-12345: a short description of the test
    ...

connector/connect/src/main/protobuf/spark/connect/expressions.proto

connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala

cloud-fan · 2022-11-14T12:13:04Z

@HyukjinKwon can you review the python side?

python/pyspark/sql/connect/column.py

python/pyspark/sql/tests/connect/test_connect_basic.py

python/pyspark/sql/tests/connect/test_connect_column_expressions.py

...tor/connect/src/test/scala/org/apache/spark/sql/connect/planner/SparkConnectProtoSuite.scala

HyukjinKwon

LGTM some nits

zhengruifeng · 2022-11-15T04:54:42Z

python/pyspark/sql/tests/connect/test_connect_column_expressions.py

+        col0 = fun.col("a").alias("martin", metadata={"pii": True})
+        plan = col0.to_plan(self.session)
+        self.assertIsNotNone(plan)
+        self.assertEqual(plan.alias.metadata, '{"pii": true}')


what about also adding a test for MultiAlias in python?

See the test above.

python/pyspark/sql/connect/dataframe.py

…ns.py Co-authored-by: Hyukjin Kwon <gurwls223@gmail.com>

…planner/SparkConnectProtoSuite.scala Co-authored-by: Hyukjin Kwon <gurwls223@gmail.com>

HyukjinKwon · 2022-11-16T09:19:56Z

Merged to master.

LuciferYang · 2022-11-22T06:24:05Z

python/pyspark/sql/tests/connect/test_connect_basic.py

+
+        with self.assertRaises(grpc.RpcError) as exc:
+            self.connect.range(1, 10).select(col("id").alias("this", "is", "not")).collect()
+        self.assertIn("Buffer(this, is, not)", str(exc.exception))


@grundprinzip The Scala 2.13 daily test has failed in recent times, which seems to be related to this assert:

====================================================================== FAIL [0.742s]: test_alias (pyspark.sql.tests.connect.test_connect_basic.SparkConnectTests) Testing supported and unsupported alias ---------------------------------------------------------------------- Traceback (most recent call last): File "/__w/spark/spark/python/pyspark/sql/tests/connect/test_connect_basic.py", line 337, in test_alias self.assertIn("Buffer(this, is, not)", str(exc.exception)) AssertionError: 'Buffer(this, is, not)' not found in '<_MultiThreadedRendezvous of RPC that terminated with:\n\tstatus = StatusCode.UNKNOWN\n\tdetails = "[INTERNAL_ERROR] Found the unresolved operator: \'Project [id#24L AS List(this, is, not)]"\n\tdebug_error_string = "UNKNOWN:Error received from peer ipv4:127.0.0.1:15002 {created_time:"2022-11-21T19:25:59.902777537+00:00", grpc_status:2, grpc_message:"[INTERNAL_ERROR] Found the unresolved operator: \\\'Project [id#24L AS List(this, is, not)]"}"\n>'

https://github.com/apache/spark/actions/runs/3517345801/jobs/5895043640

This seems to be related to the difference of Scala version. Can we simplify the assertion? such as self.assertIn("(this, is, not)", str(exc.exception))

I made a followup at #38752 :-)

…3 test pass ### What changes were proposed in this pull request? This PR is a followup of #38631 that fixes the test to pass in Scala 2.13 by avoiding using `Buffer` that becomes `List` in Scala 2.13. ### Why are the changes needed? To fix up the Scala 2.13 build, see https://github.com/apache/spark/actions/runs/3517345801/jobs/5895043640 ### Does this PR introduce _any_ user-facing change? No, test-only. ### How was this patch tested? Manually tested. Closes #38752 from HyukjinKwon/SPARK-40809-followup. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

### What changes were proposed in this pull request? This extends the implementation of column aliases in Spark Connect with supporting lists of column names and providing the appropriate implementation for the Python side. ### Why are the changes needed? Compatibility ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? UT in Python and Scala Closes apache#38631 from grundprinzip/SPARK-40809-f. Lead-authored-by: Martin Grund <martin.grund@databricks.com> Co-authored-by: Martin Grund <grundprinzip@gmail.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

…3 test pass ### What changes were proposed in this pull request? This PR is a followup of apache#38631 that fixes the test to pass in Scala 2.13 by avoiding using `Buffer` that becomes `List` in Scala 2.13. ### Why are the changes needed? To fix up the Scala 2.13 build, see https://github.com/apache/spark/actions/runs/3517345801/jobs/5895043640 ### Does this PR introduce _any_ user-facing change? No, test-only. ### How was this patch tested? Manually tested. Closes apache#38752 from HyukjinKwon/SPARK-40809-followup. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

### What changes were proposed in this pull request? This extends the implementation of column aliases in Spark Connect with supporting lists of column names and providing the appropriate implementation for the Python side. ### Why are the changes needed? Compatibility ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? UT in Python and Scala Closes apache#38631 from grundprinzip/SPARK-40809-f. Lead-authored-by: Martin Grund <martin.grund@databricks.com> Co-authored-by: Martin Grund <grundprinzip@gmail.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

…3 test pass ### What changes were proposed in this pull request? This PR is a followup of apache#38631 that fixes the test to pass in Scala 2.13 by avoiding using `Buffer` that becomes `List` in Scala 2.13. ### Why are the changes needed? To fix up the Scala 2.13 build, see https://github.com/apache/spark/actions/runs/3517345801/jobs/5895043640 ### Does this PR introduce _any_ user-facing change? No, test-only. ### How was this patch tested? Manually tested. Closes apache#38752 from HyukjinKwon/SPARK-40809-followup. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

### What changes were proposed in this pull request? This extends the implementation of column aliases in Spark Connect with supporting lists of column names and providing the appropriate implementation for the Python side. ### Why are the changes needed? Compatibility ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? UT in Python and Scala Closes apache#38631 from grundprinzip/SPARK-40809-f. Lead-authored-by: Martin Grund <martin.grund@databricks.com> Co-authored-by: Martin Grund <grundprinzip@gmail.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

…3 test pass ### What changes were proposed in this pull request? This PR is a followup of apache#38631 that fixes the test to pass in Scala 2.13 by avoiding using `Buffer` that becomes `List` in Scala 2.13. ### Why are the changes needed? To fix up the Scala 2.13 build, see https://github.com/apache/spark/actions/runs/3517345801/jobs/5895043640 ### Does this PR introduce _any_ user-facing change? No, test-only. ### How was this patch tested? Manually tested. Closes apache#38752 from HyukjinKwon/SPARK-40809-followup. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

fix alias

fb4d3fd

github-actions bot added CONNECT CORE PYTHON SQL labels Nov 11, 2022

grundprinzip changed the title ~~[SPARK-40809] [CONNECT] [FOLLOW]~~ [SPARK-40809] [CONNECT] [FOLLOW] Support alias() in Python client Nov 11, 2022

amaliujia reviewed Nov 11, 2022

View reviewed changes

grundprinzip added 4 commits November 11, 2022 22:48

adding more dsl and tests

b1d05c3

adding more dsl and tests

c2dfae6

fixing

0c9c154

format

36490c2

amaliujia reviewed Nov 13, 2022

View reviewed changes

grundprinzip added 2 commits November 13, 2022 11:01

lint

2544735

lint

0dec7ed

cloud-fan reviewed Nov 14, 2022

View reviewed changes

connector/connect/src/main/protobuf/spark/connect/expressions.proto Show resolved Hide resolved

grundprinzip and others added 2 commits November 14, 2022 07:34

lint

a6310e0

Merge branch 'apache:master' into SPARK-40809-f

9bdbe8a

cloud-fan reviewed Nov 14, 2022

View reviewed changes

connector/connect/src/main/protobuf/spark/connect/expressions.proto Show resolved Hide resolved

cloud-fan reviewed Nov 14, 2022

View reviewed changes

connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala Show resolved Hide resolved

grundprinzip added 2 commits November 14, 2022 09:58

adding scala tests

43efd68

Merge remote-tracking branch 'grundprinzip/SPARK-40809-f' into HEAD

ae31fc4

cloud-fan approved these changes Nov 14, 2022

View reviewed changes

HyukjinKwon reviewed Nov 15, 2022

View reviewed changes

python/pyspark/sql/connect/column.py Outdated Show resolved Hide resolved

HyukjinKwon reviewed Nov 15, 2022

View reviewed changes

python/pyspark/sql/tests/connect/test_connect_basic.py Outdated Show resolved Hide resolved

HyukjinKwon reviewed Nov 15, 2022

View reviewed changes

python/pyspark/sql/tests/connect/test_connect_column_expressions.py Outdated Show resolved Hide resolved

HyukjinKwon reviewed Nov 15, 2022

View reviewed changes

...tor/connect/src/test/scala/org/apache/spark/sql/connect/planner/SparkConnectProtoSuite.scala Outdated Show resolved Hide resolved

HyukjinKwon approved these changes Nov 15, 2022

View reviewed changes

zhengruifeng reviewed Nov 15, 2022

View reviewed changes

python/pyspark/sql/connect/dataframe.py Outdated Show resolved Hide resolved

grundprinzip and others added 8 commits November 15, 2022 09:39

Update python/pyspark/sql/tests/connect/test_connect_column_expressio…

b909806

…ns.py Co-authored-by: Hyukjin Kwon <gurwls223@gmail.com>

Update connector/connect/src/test/scala/org/apache/spark/sql/connect/…

b9d6fe5

…planner/SparkConnectProtoSuite.scala Co-authored-by: Hyukjin Kwon <gurwls223@gmail.com>

Update python/pyspark/sql/tests/connect/test_connect_basic.py

de3a3f7

lint

bf8f62b

Update python/pyspark/sql/connect/dataframe.py

37550b8

Merge branch 'apache:master' into SPARK-40809-f

a592c5c

fix type annotation

157b395

Merge branch 'master' into SPARK-40809-f

4937536

HyukjinKwon closed this in 0f7eaee Nov 16, 2022

HyukjinKwon mentioned this pull request Nov 22, 2022

[SPARK-40809][CONNECT][FOLLOW-UP] Do not use Buffer to make Scala 2.13 test pass #38752

Closed

LuciferYang reviewed Nov 22, 2022

View reviewed changes

[SPARK-40809] [CONNECT] [FOLLOW] Support alias() in Python client #38631

[SPARK-40809] [CONNECT] [FOLLOW] Support alias() in Python client #38631

Uh oh!

Conversation

grundprinzip commented Nov 11, 2022

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

grundprinzip commented Nov 11, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AmplabJenkins commented Nov 13, 2022

Uh oh!

amaliujia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cloud-fan commented Nov 14, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

zhengruifeng Nov 15, 2022

Choose a reason for hiding this comment

Uh oh!

grundprinzip Nov 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HyukjinKwon commented Nov 16, 2022

Uh oh!

LuciferYang Nov 22, 2022

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon Nov 22, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

[SPARK-40809] [CONNECT] [FOLLOW] Support `alias()` in Python client #38631

[SPARK-40809] [CONNECT] [FOLLOW] Support `alias()` in Python client #38631