You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "beliefer (via GitHub)" <gi...@apache.org> on 2023/03/03 07:04:40 UTC
[GitHub] [spark] beliefer opened a new pull request, #40265: [SPARK-42556][CONNECT] Dataset.colregex should link a plan_id when it only matches a single column.
beliefer opened a new pull request, #40265:
URL: https://github.com/apache/spark/pull/40265
### What changes were proposed in this pull request?
When colregex returns a single column it should link the plans plan_id. For reference here is the non-connect Dataset code that does this:
https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala#L1512
This also needs to be fixed for the Python client.
### Why are the changes needed?
Let the UnresolvedAttribute link plan_id if it is exist.
### Does this PR introduce _any_ user-facing change?
'No'.
New feature.
### How was this patch tested?
New test cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hvanhovell commented on pull request #40265: [SPARK-42556][CONNECT] Dataset.colregex should link a plan_id when it only matches a single column.
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #40265:
URL: https://github.com/apache/spark/pull/40265#issuecomment-1453443430
@beliefer can you add a test to SparkPlannerSuite or ClientE2ESuite to make sure this properly works?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hvanhovell commented on pull request #40265: [SPARK-42556][CONNECT] Dataset.colregex should link a plan_id when it only matches a single column.
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #40265:
URL: https://github.com/apache/spark/pull/40265#issuecomment-1453443869
cc @zhengruifeng can you also take a look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng closed pull request #40265: [SPARK-42556][CONNECT] Dataset.colregex should link a plan_id when it only matches a single column.
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng closed pull request #40265: [SPARK-42556][CONNECT] Dataset.colregex should link a plan_id when it only matches a single column.
URL: https://github.com/apache/spark/pull/40265
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #40265: [SPARK-42556][CONNECT] Dataset.colregex should link a plan_id when it only matches a single column.
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #40265:
URL: https://github.com/apache/spark/pull/40265#issuecomment-1454493588
merged into master/branch-3.4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hvanhovell commented on a diff in pull request #40265: [SPARK-42556][CONNECT] Dataset.colregex should link a plan_id when it only matches a single column.
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #40265:
URL: https://github.com/apache/spark/pull/40265#discussion_r1124349655
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/Dataset.scala:
##########
@@ -940,8 +940,17 @@ class Dataset[T] private[sql] (
* @group untypedrel
* @since 3.4.0
*/
- def colRegex(colName: String): Column = Column { builder =>
- builder.getUnresolvedRegexBuilder.setColName(colName)
+ def colRegex(colName: String): Column = {
+ val planId = if (plan.getRoot.hasCommon && plan.getRoot.getCommon.hasPlanId) {
Review Comment:
Perhaps put this in a helper function?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #40265: [SPARK-42556][CONNECT] Dataset.colregex should link a plan_id when it only matches a single column.
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #40265:
URL: https://github.com/apache/spark/pull/40265#issuecomment-1454533003
@hvanhovell @zhengruifeng Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #40265: [SPARK-42556][CONNECT] Dataset.colregex should link a plan_id when it only matches a single column.
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #40265:
URL: https://github.com/apache/spark/pull/40265#issuecomment-1454489494
we'd better always add e2e tests, since it was added in `ClientE2ESuite`, I think don't need to add one in `test_connect_basic`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org