You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "hiboyang (via GitHub)" <gi...@apache.org> on 2023/06/05 17:42:08 UTC
[GitHub] [spark-connect-go] hiboyang opened a new pull request, #10: Add DataFrame writer and reader prototype code
hiboyang opened a new pull request, #10:
URL: https://github.com/apache/spark-connect-go/pull/10
### What changes were proposed in this pull request?
Add dataframe writer and reader for Spark Connect Go client.
### Why are the changes needed?
This is to add more implementation in Spark Connect Go client prototype, supporting dataframe writer and reader.
### Does this PR introduce _any_ user-facing change?
Yes, user will be able to use Spark Connect Go client to write and read dataframe, like following:
```
df.Write().Mode("overwrite").
Format("parquet").
Save("file:///tmp/spark-connect-write-example-output.parquet")
df, _ = spark.Read().Format("parquet").
Load("file:///tmp/spark-connect-write-example-output.parquet")
```
### How was this patch tested?
Tested by manually run `cmd/spark-connect-example-spark-session/main.go`
--
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-connect-go] hiboyang commented on pull request #10: [spark-43351] Add DataFrame writer and reader prototype code
Posted by "hiboyang (via GitHub)" <gi...@apache.org>.
hiboyang commented on PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#issuecomment-1579817170
> @hiboyang mind creating a JIRA (https://issues.apache.org/jira/projects/SPARK)?
No problem, added JIRA https://issues.apache.org/jira/plugins/servlet/mobile#issue/SPARK-43351 in the PR description.
--
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-connect-go] hiboyang commented on pull request #10: Add DataFrame writer and reader prototype code
Posted by "hiboyang (via GitHub)" <gi...@apache.org>.
hiboyang commented on PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#issuecomment-1579578957
@HyukjinKwon @grundprinzip do you have time to take a look at this PR?
--
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-connect-go] grundprinzip commented on a diff in pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on code in PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#discussion_r1221138909
##########
client/sql/dataframereader.go:
##########
@@ -0,0 +1,41 @@
+package sql
+
+import proto "github.com/apache/spark-connect-go/v_3_4/internal/generated"
+
+type DataFrameReader interface {
Review Comment:
Same for doc here and below for the methods.
##########
client/sql/dataframe.go:
##########
@@ -31,6 +31,7 @@ type DataFrame interface {
Show(numRows int, truncate bool) error
Schema() (*StructType, error)
Collect() ([]Row, error)
+ Write() DataFrameWriter
}
type dataFrameImpl struct {
Review Comment:
I'm wondering if we should split the interfaces and the impls across multiple files for readability?
##########
client/sql/dataframereader.go:
##########
@@ -0,0 +1,41 @@
+package sql
+
+import proto "github.com/apache/spark-connect-go/v_3_4/internal/generated"
+
+type DataFrameReader interface {
+ Format(source string) DataFrameReader
+ Load(path string) (DataFrame, error)
Review Comment:
I think it's fine to create incremental PRs for the new features, but it would be great to leave a comment here showing which methods are missing in the interface.
##########
client/sql/dataframe.go:
##########
@@ -31,6 +31,7 @@ type DataFrame interface {
Show(numRows int, truncate bool) error
Schema() (*StructType, error)
Collect() ([]Row, error)
+ Write() DataFrameWriter
Review Comment:
My suggestion is to add the basic documentation (even though it's replicated from OSS Spark)
##########
client/sql/sparksession.go:
##########
@@ -154,3 +163,17 @@ func (s *sparkSessionImpl) analyzePlan(plan *proto.Plan) (*proto.AnalyzePlanResp
}
return response, nil
}
+
+func consumeExecutePlanClient(responseClient proto.SparkConnectService_ExecutePlanClient) error {
Review Comment:
Can you add a bit doc on how the data is consumed and how it deals with streaming results etc.
##########
client/sql/dataframewriter.go:
##########
@@ -0,0 +1,79 @@
+package sql
+
+import (
+ "fmt"
+ proto "github.com/apache/spark-connect-go/v_3_4/internal/generated"
+ "strings"
+)
+
+type DataFrameWriter interface {
Review Comment:
Doc :) Same comment on the missing methods for incremental changes.
--
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-connect-go] HyukjinKwon commented on pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#issuecomment-1585844887
I approved it manually for now. I will check if we can configure this in the repo level.
--
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-connect-go] zhengruifeng closed pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng closed pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
URL: https://github.com/apache/spark-connect-go/pull/10
--
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-connect-go] hiboyang commented on a diff in pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
Posted by "hiboyang (via GitHub)" <gi...@apache.org>.
hiboyang commented on code in PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#discussion_r1223289313
##########
client/sql/dataframe.go:
##########
@@ -31,6 +31,7 @@ type DataFrame interface {
Show(numRows int, truncate bool) error
Schema() (*StructType, error)
Collect() ([]Row, error)
+ Write() DataFrameWriter
Review Comment:
Good suggestion! Yes, I will update my PR with more comments/docs.
--
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-connect-go] grundprinzip commented on a diff in pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on code in PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#discussion_r1225350115
##########
cmd/spark-connect-example-spark-session/main.go:
##########
@@ -24,7 +24,7 @@ import (
)
var (
- remote = flag.String("remote", "localhost:15002",
+ remote = flag.String("remote", "sc://localhost:15002",
Review Comment:
I think this is already fixed, maybe rebase?
--
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-connect-go] zhengruifeng commented on pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#issuecomment-1592183257
merged 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-connect-go] HyukjinKwon commented on pull request #10: Add DataFrame writer and reader prototype code
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#issuecomment-1579678013
@hiboyang mind creating a JIRA (https://issues.apache.org/jira/projects/SPARK)?
--
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-connect-go] hiboyang commented on a diff in pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
Posted by "hiboyang (via GitHub)" <gi...@apache.org>.
hiboyang commented on code in PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#discussion_r1223599098
##########
client/sql/dataframe.go:
##########
@@ -31,6 +31,7 @@ type DataFrame interface {
Show(numRows int, truncate bool) error
Schema() (*StructType, error)
Collect() ([]Row, error)
+ Write() DataFrameWriter
Review Comment:
Added document.
##########
client/sql/dataframereader.go:
##########
@@ -0,0 +1,41 @@
+package sql
+
+import proto "github.com/apache/spark-connect-go/v_3_4/internal/generated"
+
+type DataFrameReader interface {
Review Comment:
Yes!
##########
client/sql/dataframereader.go:
##########
@@ -0,0 +1,41 @@
+package sql
+
+import proto "github.com/apache/spark-connect-go/v_3_4/internal/generated"
+
+type DataFrameReader interface {
+ Format(source string) DataFrameReader
+ Load(path string) (DataFrame, error)
Review Comment:
Good suggestion, added TODO for missing methods.
##########
client/sql/dataframewriter.go:
##########
@@ -0,0 +1,79 @@
+package sql
+
+import (
+ "fmt"
+ proto "github.com/apache/spark-connect-go/v_3_4/internal/generated"
+ "strings"
+)
+
+type DataFrameWriter interface {
Review Comment:
Yes!
##########
client/sql/sparksession.go:
##########
@@ -154,3 +163,17 @@ func (s *sparkSessionImpl) analyzePlan(plan *proto.Plan) (*proto.AnalyzePlanResp
}
return response, nil
}
+
+func consumeExecutePlanClient(responseClient proto.SparkConnectService_ExecutePlanClient) error {
Review Comment:
Yes, added.
--
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-connect-go] grundprinzip commented on pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#issuecomment-1585655229
@HyukjinKwon can you verify that the github action is allowed for this repository? It seems there are still issues with having maintainers to allow running it.
--
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-connect-go] hiboyang commented on a diff in pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
Posted by "hiboyang (via GitHub)" <gi...@apache.org>.
hiboyang commented on code in PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#discussion_r1223409376
##########
client/sql/dataframe.go:
##########
@@ -31,6 +31,7 @@ type DataFrame interface {
Show(numRows int, truncate bool) error
Schema() (*StructType, error)
Collect() ([]Row, error)
+ Write() DataFrameWriter
}
type dataFrameImpl struct {
Review Comment:
How about split the file when it becomes big (e.g. more than 1000 or 2000 lines)?
In Golang, it is difficult to find out which structs implement which interfaces, since Go structs does not explicitly declare what interfaces they implement (Go uses method matching to determine what interfaces a struct implements). If a file is not huge, placing interface and implementation struct in the same file will make it easy to track the implementation relationship of interfaces/structs.
--
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-connect-go] grundprinzip commented on a diff in pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on code in PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#discussion_r1225349303
##########
client/sql/dataframe.go:
##########
@@ -31,6 +31,7 @@ type DataFrame interface {
Show(numRows int, truncate bool) error
Schema() (*StructType, error)
Collect() ([]Row, error)
+ Write() DataFrameWriter
}
type dataFrameImpl struct {
Review Comment:
Sounds good. I realized the problem of the open interfaces as well :(
--
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-connect-go] zhengruifeng commented on pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#issuecomment-1585426892
it seems that the `build & test` is not automatically triggered by commit?
--
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-connect-go] hiboyang commented on a diff in pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
Posted by "hiboyang (via GitHub)" <gi...@apache.org>.
hiboyang commented on code in PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#discussion_r1225617752
##########
cmd/spark-connect-example-spark-session/main.go:
##########
@@ -24,7 +24,7 @@ import (
)
var (
- remote = flag.String("remote", "localhost:15002",
+ remote = flag.String("remote", "sc://localhost:15002",
Review Comment:
You are right! Did rebase just now.
--
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-connect-go] hiboyang commented on pull request #10: [SPARK-43351] Add DataFrame writer and reader prototype code
Posted by "hiboyang (via GitHub)" <gi...@apache.org>.
hiboyang commented on PR #10:
URL: https://github.com/apache/spark-connect-go/pull/10#issuecomment-1591472566
@grundprinzip would you help to check this PR to see whether it is good for merge?
--
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