You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "hvanhovell (via GitHub)" <gi...@apache.org> on 2023/08/15 00:42:10 UTC
[GitHub] [spark] hvanhovell opened a new pull request, #42492: [SPARK-44807][CONNECT] Add Dataset.metadataColumn to Scala Client
hvanhovell opened a new pull request, #42492:
URL: https://github.com/apache/spark/pull/42492
### What changes were proposed in this pull request?
This PR adds Dataset.metadataColumn to the Spark Connect Scala Client.
### Why are the changes needed?
We want the scala client to be as compatible as possible with the API provided by sql/core.
### Does this PR introduce _any_ user-facing change?
Yes, it adds a new method to the Spark Connect Scala Client.
### How was this patch tested?
I added a test to `ClientE2ETestSuite`.
--
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 #42492: [SPARK-44807][CONNECT] Add Dataset.metadataColumn to Scala Client
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #42492:
URL: https://github.com/apache/spark/pull/42492#issuecomment-1678273313
For the reviewers, I am not sure this is the best approach...
--
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 closed pull request #42492: [SPARK-44807][CONNECT] Add Dataset.metadataColumn to Scala Client
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell closed pull request #42492: [SPARK-44807][CONNECT] Add Dataset.metadataColumn to Scala Client
URL: https://github.com/apache/spark/pull/42492
--
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 #42492: [SPARK-44807][CONNECT] Add Dataset.metadataColumn to Scala Client
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #42492:
URL: https://github.com/apache/spark/pull/42492#issuecomment-1680899983
Merging to master.
--
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 #42492: [SPARK-44807][CONNECT] Add Dataset.metadataColumn to Scala Client
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #42492:
URL: https://github.com/apache/spark/pull/42492#issuecomment-1679418195
@cloud-fan PTAL
--
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 #42492: [SPARK-44807][CONNECT] Add Dataset.metadataColumn to Scala Client
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #42492:
URL: https://github.com/apache/spark/pull/42492#discussion_r1294088333
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -1403,6 +1403,9 @@ class SparkConnectPlanner(val sessionHolder: SessionHolder) extends Logging {
if (attr.hasPlanId) {
expr.setTagValue(LogicalPlan.PLAN_ID_TAG, attr.getPlanId)
}
+ if (attr.hasIsMetadataColumn) {
Review Comment:
For the record I am not sure I like this. It gets the job done, but I do feel we should use actual fields for this instead of sneaking it in.
--
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