You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/12/15 15:57:31 UTC
[GitHub] [spark] vicennial opened a new pull request, #39078: [SPARK-41534] Setup initial client module for Spark Connect
vicennial opened a new pull request, #39078:
URL: https://github.com/apache/spark/pull/39078
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'core/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
This PR sets up the client module (primarily the build for SBT and Maven) in Spark Connect in preparation for the Scala client. Placeholder `SparkSession` and `Dataset` files are added to test the build.
Note: A dependency to `sql-catalyst` is added as a **temporary measure** until the few common dependencies that the client needs are extracted out.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
For development of the Scala client for Spark Connect.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
Existing build CI.
--
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] dongjoon-hyun commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1051251797
##########
connector/connect/client/pom.xml:
##########
@@ -0,0 +1,112 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one or more
+ ~ contributor license agreements. See the NOTICE file distributed with
+ ~ this work for additional information regarding copyright ownership.
+ ~ The ASF licenses this file to You under the Apache License, Version 2.0
+ ~ (the "License"); you may not use this file except in compliance with
+ ~ the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.spark</groupId>
+ <artifactId>spark-parent_2.12</artifactId>
+ <version>3.4.0-SNAPSHOT</version>
+ <relativePath>../../../pom.xml</relativePath>
+ </parent>
+
+ <artifactId>spark-connect-client_2.12</artifactId>
+ <packaging>jar</packaging>
+ <name>Spark Project Connect Client</name>
+ <url>https://spark.apache.org/</url>
+ <properties>
+ <sbt.project.name>connect-client</sbt.project.name>
+ <guava.version>31.0.1-jre</guava.version>
+ <guava.failureaccess.version>1.0.1</guava.failureaccess.version>
+ <io.grpc.version>1.47.0</io.grpc.version>
+ <tomcat.annotations.api.version>6.0.53</tomcat.annotations.api.version>
+ </properties>
Review Comment:
This seems to be copied from `common` module. Could you try to avoid this kind of duplication?
https://github.com/apache/spark/blob/f3be74e3a421224bb65a01e658de96006de2a4ac/connector/connect/common/pom.xml#L33-L39
--
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] amaliujia commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
amaliujia commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1052834300
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
+ private val userContext: proto.UserContext,
+ private val channel: ManagedChannel)
+ extends AutoCloseable {
+ private[this] val stub = proto.SparkConnectServiceGrpc.newBlockingStub(channel)
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 2.0.0
+ */
+ def range(start: Long, end: Long, step: Long, numPartitions: Int): Dataset = {
+ range(start, end, step, Option(numPartitions))
+ }
+
+ private def range(start: Long, end: Long, step: Long, numPartitions: Option[Int]): Dataset = {
+ newDataset { builder =>
+ val rangeBuilder = builder.getRangeBuilder
+ .setStart(start)
+ .setEnd(end)
+ .setStep(step)
+ numPartitions.foreach(rangeBuilder.setNumPartitions)
+ }
+ }
+
+ /**
+ * Executes a SQL query using Spark, returning the result as a `DataFrame`.
+ * This API eagerly runs DDL/DML commands, but not for SELECT queries.
+ *
+ * @since 2.0.0
+ */
+ def sql(query: String): Dataset = newDataset { builder =>
+ builder.setSql(proto.SQL.newBuilder().setQuery(query))
+ }
+
+ private[client] def newDataset(f: proto.Relation.Builder => Unit): Dataset = {
+ val builder = proto.Relation.newBuilder()
+ f(builder)
+ val plan = proto.Plan.newBuilder().setRoot(builder).build()
+ new Dataset(this, plan)
+ }
+
+ private[client] def analyze(plan: proto.Plan): proto.AnalyzePlanResponse = {
+ val request = proto.AnalyzePlanRequest.newBuilder()
+ .setPlan(plan)
+ .setUserContext(userContext)
+ .build()
+ stub.analyzePlan(request)
+ }
+
+ override def close(): Unit = {
+ channel.shutdownNow()
+ allocator.close()
+ }
+}
+
+object SparkSession {
+ def builder(): Builder = new Builder()
+
+ class Builder() {
Review Comment:
> I also don't quite understand the comment about the connection string. That is just an option we should support (URI parsing is very much a solved problem).
I am not sure if it is an option or a must. Different clients do not share a standard way to connect to server does not sound right.
We may not need it in the first version though. If ultimately we agree that all clients should follow the same way for the connection, at least there is a JIRA to track or a TODO here to say Scala side will follow the connection string is good.
--
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] grundprinzip commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1053291093
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
+ private val userContext: proto.UserContext,
+ private val channel: ManagedChannel)
+ extends AutoCloseable {
+ private[this] val stub = proto.SparkConnectServiceGrpc.newBlockingStub(channel)
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 2.0.0
+ */
+ def range(start: Long, end: Long, step: Long, numPartitions: Int): Dataset = {
+ range(start, end, step, Option(numPartitions))
+ }
+
+ private def range(start: Long, end: Long, step: Long, numPartitions: Option[Int]): Dataset = {
+ newDataset { builder =>
+ val rangeBuilder = builder.getRangeBuilder
+ .setStart(start)
+ .setEnd(end)
+ .setStep(step)
+ numPartitions.foreach(rangeBuilder.setNumPartitions)
+ }
+ }
+
+ /**
+ * Executes a SQL query using Spark, returning the result as a `DataFrame`.
+ * This API eagerly runs DDL/DML commands, but not for SELECT queries.
+ *
+ * @since 2.0.0
+ */
+ def sql(query: String): Dataset = newDataset { builder =>
+ builder.setSql(proto.SQL.newBuilder().setQuery(query))
+ }
+
+ private[client] def newDataset(f: proto.Relation.Builder => Unit): Dataset = {
+ val builder = proto.Relation.newBuilder()
+ f(builder)
+ val plan = proto.Plan.newBuilder().setRoot(builder).build()
+ new Dataset(this, plan)
+ }
+
+ private[client] def analyze(plan: proto.Plan): proto.AnalyzePlanResponse = {
+ val request = proto.AnalyzePlanRequest.newBuilder()
+ .setPlan(plan)
+ .setUserContext(userContext)
+ .build()
+ stub.analyzePlan(request)
+ }
+
+ override def close(): Unit = {
+ channel.shutdownNow()
+ allocator.close()
+ }
+}
+
+object SparkSession {
+ def builder(): Builder = new Builder()
+
+ class Builder() {
Review Comment:
@hvanhovell that was the point with the suggestion to start the PR without the GRPC client at all.
--
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 #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
hvanhovell closed pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
URL: https://github.com/apache/spark/pull/39078
--
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] HyukjinKwon commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1050319465
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
+ private val userContext: proto.UserContext,
+ private val channel: ManagedChannel)
+ extends AutoCloseable {
+ private[this] val stub = proto.SparkConnectServiceGrpc.newBlockingStub(channel)
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 2.0.0
Review Comment:
```suggestion
* @since 3.4.0
```
--
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] amaliujia commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
amaliujia commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1052698977
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
Review Comment:
great +1 on this.
--
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] dongjoon-hyun commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1051250909
##########
connector/connect/client/pom.xml:
##########
@@ -0,0 +1,112 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one or more
+ ~ contributor license agreements. See the NOTICE file distributed with
+ ~ this work for additional information regarding copyright ownership.
+ ~ The ASF licenses this file to You under the Apache License, Version 2.0
+ ~ (the "License"); you may not use this file except in compliance with
+ ~ the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
Review Comment:
If you don't mind, please use 2-space indentation in `pom.xml` like the parent pom file.
https://github.com/apache/spark/blob/f3be74e3a421224bb65a01e658de96006de2a4ac/pom.xml#L19-L27
--
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] amaliujia commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
amaliujia commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1052725981
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
+ private val userContext: proto.UserContext,
+ private val channel: ManagedChannel)
+ extends AutoCloseable {
+ private[this] val stub = proto.SparkConnectServiceGrpc.newBlockingStub(channel)
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 2.0.0
+ */
+ def range(start: Long, end: Long, step: Long, numPartitions: Int): Dataset = {
+ range(start, end, step, Option(numPartitions))
+ }
+
+ private def range(start: Long, end: Long, step: Long, numPartitions: Option[Int]): Dataset = {
+ newDataset { builder =>
+ val rangeBuilder = builder.getRangeBuilder
+ .setStart(start)
+ .setEnd(end)
+ .setStep(step)
+ numPartitions.foreach(rangeBuilder.setNumPartitions)
+ }
+ }
+
+ /**
+ * Executes a SQL query using Spark, returning the result as a `DataFrame`.
+ * This API eagerly runs DDL/DML commands, but not for SELECT queries.
+ *
+ * @since 2.0.0
+ */
+ def sql(query: String): Dataset = newDataset { builder =>
+ builder.setSql(proto.SQL.newBuilder().setQuery(query))
+ }
+
+ private[client] def newDataset(f: proto.Relation.Builder => Unit): Dataset = {
+ val builder = proto.Relation.newBuilder()
+ f(builder)
+ val plan = proto.Plan.newBuilder().setRoot(builder).build()
+ new Dataset(this, plan)
+ }
+
+ private[client] def analyze(plan: proto.Plan): proto.AnalyzePlanResponse = {
+ val request = proto.AnalyzePlanRequest.newBuilder()
+ .setPlan(plan)
+ .setUserContext(userContext)
+ .build()
+ stub.analyzePlan(request)
+ }
+
+ override def close(): Unit = {
+ channel.shutdownNow()
+ allocator.close()
+ }
+}
+
+object SparkSession {
+ def builder(): Builder = new Builder()
+
+ class Builder() {
Review Comment:
yeah support ChannelBuilder probably means that you should support https://github.com/apache/spark/blob/master/connector/connect/docs/client-connection-string.md
--
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] amaliujia commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
amaliujia commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1052834300
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
+ private val userContext: proto.UserContext,
+ private val channel: ManagedChannel)
+ extends AutoCloseable {
+ private[this] val stub = proto.SparkConnectServiceGrpc.newBlockingStub(channel)
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 2.0.0
+ */
+ def range(start: Long, end: Long, step: Long, numPartitions: Int): Dataset = {
+ range(start, end, step, Option(numPartitions))
+ }
+
+ private def range(start: Long, end: Long, step: Long, numPartitions: Option[Int]): Dataset = {
+ newDataset { builder =>
+ val rangeBuilder = builder.getRangeBuilder
+ .setStart(start)
+ .setEnd(end)
+ .setStep(step)
+ numPartitions.foreach(rangeBuilder.setNumPartitions)
+ }
+ }
+
+ /**
+ * Executes a SQL query using Spark, returning the result as a `DataFrame`.
+ * This API eagerly runs DDL/DML commands, but not for SELECT queries.
+ *
+ * @since 2.0.0
+ */
+ def sql(query: String): Dataset = newDataset { builder =>
+ builder.setSql(proto.SQL.newBuilder().setQuery(query))
+ }
+
+ private[client] def newDataset(f: proto.Relation.Builder => Unit): Dataset = {
+ val builder = proto.Relation.newBuilder()
+ f(builder)
+ val plan = proto.Plan.newBuilder().setRoot(builder).build()
+ new Dataset(this, plan)
+ }
+
+ private[client] def analyze(plan: proto.Plan): proto.AnalyzePlanResponse = {
+ val request = proto.AnalyzePlanRequest.newBuilder()
+ .setPlan(plan)
+ .setUserContext(userContext)
+ .build()
+ stub.analyzePlan(request)
+ }
+
+ override def close(): Unit = {
+ channel.shutdownNow()
+ allocator.close()
+ }
+}
+
+object SparkSession {
+ def builder(): Builder = new Builder()
+
+ class Builder() {
Review Comment:
> I also don't quite understand the comment about the connection string. That is just an option we should support (URI parsing is very much a solved problem).
I am not sure if it is an option or a must. Different clients do not share a standard way to connect to server does not sound right.
We may not need it in the first version though.
--
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] grundprinzip commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1053290375
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
Review Comment:
There is a difference in testing the GRPC client and the SparkSession, for some parts we inject a mock client into the session because we don't want to or cannot setup the client.
That's just a very reasonable separation of concerns. The SparkSession deals with the SparkSide, while the client deals with the proto side.
--
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] amaliujia commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
amaliujia commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1052725694
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
+ private val userContext: proto.UserContext,
+ private val channel: ManagedChannel)
+ extends AutoCloseable {
+ private[this] val stub = proto.SparkConnectServiceGrpc.newBlockingStub(channel)
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 2.0.0
+ */
+ def range(start: Long, end: Long, step: Long, numPartitions: Int): Dataset = {
+ range(start, end, step, Option(numPartitions))
+ }
+
+ private def range(start: Long, end: Long, step: Long, numPartitions: Option[Int]): Dataset = {
+ newDataset { builder =>
+ val rangeBuilder = builder.getRangeBuilder
+ .setStart(start)
+ .setEnd(end)
+ .setStep(step)
+ numPartitions.foreach(rangeBuilder.setNumPartitions)
+ }
+ }
+
+ /**
+ * Executes a SQL query using Spark, returning the result as a `DataFrame`.
+ * This API eagerly runs DDL/DML commands, but not for SELECT queries.
+ *
+ * @since 2.0.0
+ */
+ def sql(query: String): Dataset = newDataset { builder =>
+ builder.setSql(proto.SQL.newBuilder().setQuery(query))
+ }
+
+ private[client] def newDataset(f: proto.Relation.Builder => Unit): Dataset = {
+ val builder = proto.Relation.newBuilder()
+ f(builder)
+ val plan = proto.Plan.newBuilder().setRoot(builder).build()
+ new Dataset(this, plan)
+ }
+
+ private[client] def analyze(plan: proto.Plan): proto.AnalyzePlanResponse = {
+ val request = proto.AnalyzePlanRequest.newBuilder()
+ .setPlan(plan)
+ .setUserContext(userContext)
+ .build()
+ stub.analyzePlan(request)
+ }
+
+ override def close(): Unit = {
+ channel.shutdownNow()
+ allocator.close()
+ }
+}
+
+object SparkSession {
+ def builder(): Builder = new Builder()
+
+ class Builder() {
+ private val userContextBuilder = proto.UserContext.newBuilder()
+ private var _host: String = "localhost"
+ private var _port: Int = 15002
Review Comment:
+1 for this
--
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] amaliujia commented on pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
amaliujia commented on PR #39078:
URL: https://github.com/apache/spark/pull/39078#issuecomment-1363210209
LGTM thanks for simplifying the dependencies!
--
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] amaliujia commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
amaliujia commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1051078343
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
+ private val userContext: proto.UserContext,
+ private val channel: ManagedChannel)
+ extends AutoCloseable {
+ private[this] val stub = proto.SparkConnectServiceGrpc.newBlockingStub(channel)
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 2.0.0
Review Comment:
The client starts from 3.4.0?
--
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] dongjoon-hyun commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1051252600
##########
project/SparkBuild.scala:
##########
@@ -1433,7 +1492,10 @@ object CopyDependencies {
if (jar.getName.contains("spark-connect") &&
!SbtPomKeys.profiles.value.contains("noshade-connect")) {
Files.copy(fid.toPath, destJar.toPath)
- } else if (jar.getName.contains("spark-protobuf") &&
+ } else if (jar.getName.contains("connect-client") &&
+ !SbtPomKeys.profiles.value.contains("noshade-protobuf")) {
+ Files.copy(fidClient.toPath, destJar.toPath)
+ } else if (jar.getName.contains("spark-protobuf") &&
Review Comment:
Please remove an extra space, `} else` -> `} else`.
--
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] grundprinzip commented on pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on PR #39078:
URL: https://github.com/apache/spark/pull/39078#issuecomment-1359902896
Yes. My main point is that the pom.xml is at the root of the folder and most other systems will want to have their own build systems and sub folder structure.
Just up everything by one 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] hvanhovell commented on pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
hvanhovell commented on PR #39078:
URL: https://github.com/apache/spark/pull/39078#issuecomment-1359717072
@grundprinzip what other clients will be using this directory? I mean an R client will be written in R right? Same for Rust?
--
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 #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
hvanhovell commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1052790753
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
+ private val userContext: proto.UserContext,
+ private val channel: ManagedChannel)
+ extends AutoCloseable {
+ private[this] val stub = proto.SparkConnectServiceGrpc.newBlockingStub(channel)
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 2.0.0
+ */
+ def range(start: Long, end: Long, step: Long, numPartitions: Int): Dataset = {
+ range(start, end, step, Option(numPartitions))
+ }
+
+ private def range(start: Long, end: Long, step: Long, numPartitions: Option[Int]): Dataset = {
+ newDataset { builder =>
+ val rangeBuilder = builder.getRangeBuilder
+ .setStart(start)
+ .setEnd(end)
+ .setStep(step)
+ numPartitions.foreach(rangeBuilder.setNumPartitions)
+ }
+ }
+
+ /**
+ * Executes a SQL query using Spark, returning the result as a `DataFrame`.
+ * This API eagerly runs DDL/DML commands, but not for SELECT queries.
+ *
+ * @since 2.0.0
+ */
+ def sql(query: String): Dataset = newDataset { builder =>
+ builder.setSql(proto.SQL.newBuilder().setQuery(query))
+ }
+
+ private[client] def newDataset(f: proto.Relation.Builder => Unit): Dataset = {
+ val builder = proto.Relation.newBuilder()
+ f(builder)
+ val plan = proto.Plan.newBuilder().setRoot(builder).build()
+ new Dataset(this, plan)
+ }
+
+ private[client] def analyze(plan: proto.Plan): proto.AnalyzePlanResponse = {
+ val request = proto.AnalyzePlanRequest.newBuilder()
+ .setPlan(plan)
+ .setUserContext(userContext)
+ .build()
+ stub.analyzePlan(request)
+ }
+
+ override def close(): Unit = {
+ channel.shutdownNow()
+ allocator.close()
+ }
+}
+
+object SparkSession {
+ def builder(): Builder = new Builder()
+
+ class Builder() {
Review Comment:
We use a channel builder in the build method?
I also don't quite understand the comment about the connection string. That is just an option we should support (URI parsing is very much a solved problem).
Finally, sure we can make this PR smaller. It is early days for the Scala Client and we need to start somewhere.
--
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] vicennial commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
vicennial commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1053268138
##########
connector/connect/client/pom.xml:
##########
@@ -0,0 +1,112 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one or more
+ ~ contributor license agreements. See the NOTICE file distributed with
+ ~ this work for additional information regarding copyright ownership.
+ ~ The ASF licenses this file to You under the Apache License, Version 2.0
+ ~ (the "License"); you may not use this file except in compliance with
+ ~ the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.spark</groupId>
+ <artifactId>spark-parent_2.12</artifactId>
+ <version>3.4.0-SNAPSHOT</version>
+ <relativePath>../../../pom.xml</relativePath>
+ </parent>
+
+ <artifactId>spark-connect-client_2.12</artifactId>
+ <packaging>jar</packaging>
+ <name>Spark Project Connect Client</name>
+ <url>https://spark.apache.org/</url>
+ <properties>
+ <sbt.project.name>connect-client</sbt.project.name>
+ <guava.version>31.0.1-jre</guava.version>
+ <guava.failureaccess.version>1.0.1</guava.failureaccess.version>
+ <io.grpc.version>1.47.0</io.grpc.version>
+ <tomcat.annotations.api.version>6.0.53</tomcat.annotations.api.version>
+ </properties>
Review Comment:
Apologies, updated.
--
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 #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
hvanhovell commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1052791191
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
Review Comment:
Why? What do you want to test?
--
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 #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
hvanhovell commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1053317891
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
+ private val userContext: proto.UserContext,
+ private val channel: ManagedChannel)
+ extends AutoCloseable {
+ private[this] val stub = proto.SparkConnectServiceGrpc.newBlockingStub(channel)
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 2.0.0
+ */
+ def range(start: Long, end: Long, step: Long, numPartitions: Int): Dataset = {
+ range(start, end, step, Option(numPartitions))
+ }
+
+ private def range(start: Long, end: Long, step: Long, numPartitions: Option[Int]): Dataset = {
+ newDataset { builder =>
+ val rangeBuilder = builder.getRangeBuilder
+ .setStart(start)
+ .setEnd(end)
+ .setStep(step)
+ numPartitions.foreach(rangeBuilder.setNumPartitions)
+ }
+ }
+
+ /**
+ * Executes a SQL query using Spark, returning the result as a `DataFrame`.
+ * This API eagerly runs DDL/DML commands, but not for SELECT queries.
+ *
+ * @since 2.0.0
+ */
+ def sql(query: String): Dataset = newDataset { builder =>
+ builder.setSql(proto.SQL.newBuilder().setQuery(query))
+ }
+
+ private[client] def newDataset(f: proto.Relation.Builder => Unit): Dataset = {
+ val builder = proto.Relation.newBuilder()
+ f(builder)
+ val plan = proto.Plan.newBuilder().setRoot(builder).build()
+ new Dataset(this, plan)
+ }
+
+ private[client] def analyze(plan: proto.Plan): proto.AnalyzePlanResponse = {
+ val request = proto.AnalyzePlanRequest.newBuilder()
+ .setPlan(plan)
+ .setUserContext(userContext)
+ .build()
+ stub.analyzePlan(request)
+ }
+
+ override def close(): Unit = {
+ channel.shutdownNow()
+ allocator.close()
+ }
+}
+
+object SparkSession {
+ def builder(): Builder = new Builder()
+
+ class Builder() {
Review Comment:
IMO we are burning a lot of cycles on code that is just meant to check if the project properly compiles.
--
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] grundprinzip commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1053333910
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(private val userContext: proto.UserContext) extends AutoCloseable {
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 3.4.0
+ */
+ def range(start: Long, end: Long, step: Long, numPartitions: Int): Dataset = {
+ range(start, end, step, Option(numPartitions))
+ }
+
+ private def range(start: Long, end: Long, step: Long, numPartitions: Option[Int]): Dataset = {
+ newDataset { builder =>
+ val rangeBuilder = builder.getRangeBuilder
+ .setStart(start)
+ .setEnd(end)
+ .setStep(step)
+ numPartitions.foreach(rangeBuilder.setNumPartitions)
+ }
+ }
+
+ /**
+ * Executes a SQL query using Spark, returning the result as a `DataFrame`.
+ * This API eagerly runs DDL/DML commands, but not for SELECT queries.
+ *
+ * @since 3.4.0
+ */
+ def sql(query: String): Dataset = newDataset { builder =>
+ builder.setSql(proto.SQL.newBuilder().setQuery(query))
Review Comment:
just as an FYI because we have the same bug in Python. Right now this API actually is not eagerly evaluating the SQL query.
--
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 #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
hvanhovell commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1053335826
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(private val userContext: proto.UserContext) extends AutoCloseable {
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 3.4.0
+ */
+ def range(start: Long, end: Long, step: Long, numPartitions: Int): Dataset = {
+ range(start, end, step, Option(numPartitions))
+ }
+
+ private def range(start: Long, end: Long, step: Long, numPartitions: Option[Int]): Dataset = {
+ newDataset { builder =>
+ val rangeBuilder = builder.getRangeBuilder
+ .setStart(start)
+ .setEnd(end)
+ .setStep(step)
+ numPartitions.foreach(rangeBuilder.setNumPartitions)
+ }
+ }
+
+ /**
+ * Executes a SQL query using Spark, returning the result as a `DataFrame`.
+ * This API eagerly runs DDL/DML commands, but not for SELECT queries.
+ *
+ * @since 3.4.0
+ */
+ def sql(query: String): Dataset = newDataset { builder =>
+ builder.setSql(proto.SQL.newBuilder().setQuery(query))
Review Comment:
That is only an issue for commands right?
--
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] vicennial commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
vicennial commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1053268590
##########
connector/connect/client/pom.xml:
##########
@@ -0,0 +1,112 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one or more
+ ~ contributor license agreements. See the NOTICE file distributed with
+ ~ this work for additional information regarding copyright ownership.
+ ~ The ASF licenses this file to You under the Apache License, Version 2.0
+ ~ (the "License"); you may not use this file except in compliance with
+ ~ the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
Review Comment:
Ah, my ide was defaulting to 4-space, updated the file and settings 👍
--
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] vicennial commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
vicennial commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1053267806
##########
project/SparkBuild.scala:
##########
@@ -1433,7 +1492,10 @@ object CopyDependencies {
if (jar.getName.contains("spark-connect") &&
!SbtPomKeys.profiles.value.contains("noshade-connect")) {
Files.copy(fid.toPath, destJar.toPath)
- } else if (jar.getName.contains("spark-protobuf") &&
+ } else if (jar.getName.contains("connect-client") &&
+ !SbtPomKeys.profiles.value.contains("noshade-protobuf")) {
Review Comment:
Thanks, there was a typo. Fixed.
--
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] grundprinzip commented on pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on PR #39078:
URL: https://github.com/apache/spark/pull/39078#issuecomment-1359395029
Generally, I'm fine with merging and iterating from here. However, one last question to @hvanhovell and @vicennial. Can you please move the client into a subdirectory of `connector/connect/client/scala/` rather than in the generic client directory. This gives more room for other clients to find their place.
--
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] AmplabJenkins commented on pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #39078:
URL: https://github.com/apache/spark/pull/39078#issuecomment-1356658685
Can one of the admins verify this patch?
--
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 #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
hvanhovell commented on PR #39078:
URL: https://github.com/apache/spark/pull/39078#issuecomment-1363197466
merging this
--
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] grundprinzip commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1052679741
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
Review Comment:
From our work in the Python client it turned out that it's actually better to keep the GRPC client out of the `SparkSession` and have a separate `SparkConnectClient` class for that. That gives you as well better testability.
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
+ private val userContext: proto.UserContext,
+ private val channel: ManagedChannel)
+ extends AutoCloseable {
+ private[this] val stub = proto.SparkConnectServiceGrpc.newBlockingStub(channel)
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 2.0.0
+ */
+ def range(start: Long, end: Long, step: Long, numPartitions: Int): Dataset = {
+ range(start, end, step, Option(numPartitions))
+ }
+
+ private def range(start: Long, end: Long, step: Long, numPartitions: Option[Int]): Dataset = {
+ newDataset { builder =>
+ val rangeBuilder = builder.getRangeBuilder
+ .setStart(start)
+ .setEnd(end)
+ .setStep(step)
+ numPartitions.foreach(rangeBuilder.setNumPartitions)
+ }
+ }
+
+ /**
+ * Executes a SQL query using Spark, returning the result as a `DataFrame`.
+ * This API eagerly runs DDL/DML commands, but not for SELECT queries.
+ *
+ * @since 2.0.0
+ */
+ def sql(query: String): Dataset = newDataset { builder =>
+ builder.setSql(proto.SQL.newBuilder().setQuery(query))
+ }
+
+ private[client] def newDataset(f: proto.Relation.Builder => Unit): Dataset = {
+ val builder = proto.Relation.newBuilder()
+ f(builder)
+ val plan = proto.Plan.newBuilder().setRoot(builder).build()
+ new Dataset(this, plan)
+ }
+
+ private[client] def analyze(plan: proto.Plan): proto.AnalyzePlanResponse = {
+ val request = proto.AnalyzePlanRequest.newBuilder()
+ .setPlan(plan)
+ .setUserContext(userContext)
+ .build()
+ stub.analyzePlan(request)
+ }
+
+ override def close(): Unit = {
+ channel.shutdownNow()
+ allocator.close()
+ }
+}
+
+object SparkSession {
+ def builder(): Builder = new Builder()
+
+ class Builder() {
Review Comment:
It would be good to use a similar `ChannelBuilder` construct as in the Python client to have the same behavior on both sides. I understand this is kind of painful to do for the first PR. Maybe the alternative is to make the first PR even smaller.
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
+ private val userContext: proto.UserContext,
+ private val channel: ManagedChannel)
+ extends AutoCloseable {
+ private[this] val stub = proto.SparkConnectServiceGrpc.newBlockingStub(channel)
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 2.0.0
+ */
+ def range(start: Long, end: Long, step: Long, numPartitions: Int): Dataset = {
+ range(start, end, step, Option(numPartitions))
+ }
+
+ private def range(start: Long, end: Long, step: Long, numPartitions: Option[Int]): Dataset = {
+ newDataset { builder =>
+ val rangeBuilder = builder.getRangeBuilder
+ .setStart(start)
+ .setEnd(end)
+ .setStep(step)
+ numPartitions.foreach(rangeBuilder.setNumPartitions)
+ }
+ }
+
+ /**
+ * Executes a SQL query using Spark, returning the result as a `DataFrame`.
+ * This API eagerly runs DDL/DML commands, but not for SELECT queries.
+ *
+ * @since 2.0.0
Review Comment:
```suggestion
* @since 3.4.0
```
--
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 #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
hvanhovell commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1052791191
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
Review Comment:
Why? What else do you want to test?
--
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 #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
hvanhovell commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1053317253
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
+ private val userContext: proto.UserContext,
+ private val channel: ManagedChannel)
+ extends AutoCloseable {
+ private[this] val stub = proto.SparkConnectServiceGrpc.newBlockingStub(channel)
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 2.0.0
+ */
+ def range(start: Long, end: Long, step: Long, numPartitions: Int): Dataset = {
+ range(start, end, step, Option(numPartitions))
+ }
+
+ private def range(start: Long, end: Long, step: Long, numPartitions: Option[Int]): Dataset = {
+ newDataset { builder =>
+ val rangeBuilder = builder.getRangeBuilder
+ .setStart(start)
+ .setEnd(end)
+ .setStep(step)
+ numPartitions.foreach(rangeBuilder.setNumPartitions)
+ }
+ }
+
+ /**
+ * Executes a SQL query using Spark, returning the result as a `DataFrame`.
+ * This API eagerly runs DDL/DML commands, but not for SELECT queries.
+ *
+ * @since 2.0.0
+ */
+ def sql(query: String): Dataset = newDataset { builder =>
+ builder.setSql(proto.SQL.newBuilder().setQuery(query))
+ }
+
+ private[client] def newDataset(f: proto.Relation.Builder => Unit): Dataset = {
+ val builder = proto.Relation.newBuilder()
+ f(builder)
+ val plan = proto.Plan.newBuilder().setRoot(builder).build()
+ new Dataset(this, plan)
+ }
+
+ private[client] def analyze(plan: proto.Plan): proto.AnalyzePlanResponse = {
+ val request = proto.AnalyzePlanRequest.newBuilder()
+ .setPlan(plan)
+ .setUserContext(userContext)
+ .build()
+ stub.analyzePlan(request)
+ }
+
+ override def close(): Unit = {
+ channel.shutdownNow()
+ allocator.close()
+ }
+}
+
+object SparkSession {
+ def builder(): Builder = new Builder()
+
+ class Builder() {
Review Comment:
@amaliujia I mean it is option during configuration. I didn't mean it is optional. We will add 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] vicennial commented on pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
vicennial commented on PR #39078:
URL: https://github.com/apache/spark/pull/39078#issuecomment-1361296845
@grundprinzip Looks like there are two options here if we want separate folders:
- `connect/client/[scala/xyz]`
- `connect/[scala/xyz]-client`
WDYT about the latter option? That would keep clients isolated and it avoids the misnomer in `connect/client/scala` that makes it wrongfully imply that it's a language directory rather than a client.
--
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] vicennial commented on pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
vicennial commented on PR #39078:
URL: https://github.com/apache/spark/pull/39078#issuecomment-1361300713
There is also `connect/client/jvm` that @hvanhovell suggested for the former option so we can avoid using the term "scala"
--
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] dongjoon-hyun commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1051253194
##########
connector/connect/client/src/main/scala/org/apache/spark/sql/connect/client/SparkSession.scala:
##########
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connect.client
+
+import scala.language.existentials
+
+import io.grpc.{ManagedChannel, ManagedChannelBuilder}
+import org.apache.arrow.memory.RootAllocator
+
+import org.apache.spark.SPARK_VERSION
+import org.apache.spark.connect.proto
+
+
+class SparkSession(
+ private val userContext: proto.UserContext,
+ private val channel: ManagedChannel)
+ extends AutoCloseable {
+ private[this] val stub = proto.SparkConnectServiceGrpc.newBlockingStub(channel)
+
+ private[this] val allocator = new RootAllocator()
+
+ /**
+ * The version of Spark on which this application is running.
+ */
+ def version: String = SPARK_VERSION
+
+ /**
+ * Returns a `DataFrame` with no rows or columns.
+ *
+ * @since 3.4.0
+ */
+ @transient
+ lazy val emptyDataFrame: Dataset = newDataset { builder =>
+ builder.getLocalRelationBuilder
+ }
+
+ /**
+ * Creates a [[Dataset]] with a single `LongType` column named `id`, containing elements
+ * in a range from `start` to `end` (exclusive) with a step value, with partition number
+ * specified.
+ *
+ * @since 2.0.0
+ */
+ def range(start: Long, end: Long, step: Long, numPartitions: Int): Dataset = {
+ range(start, end, step, Option(numPartitions))
+ }
+
+ private def range(start: Long, end: Long, step: Long, numPartitions: Option[Int]): Dataset = {
+ newDataset { builder =>
+ val rangeBuilder = builder.getRangeBuilder
+ .setStart(start)
+ .setEnd(end)
+ .setStep(step)
+ numPartitions.foreach(rangeBuilder.setNumPartitions)
+ }
+ }
+
+ /**
+ * Executes a SQL query using Spark, returning the result as a `DataFrame`.
+ * This API eagerly runs DDL/DML commands, but not for SELECT queries.
+ *
+ * @since 2.0.0
+ */
+ def sql(query: String): Dataset = newDataset { builder =>
+ builder.setSql(proto.SQL.newBuilder().setQuery(query))
+ }
+
+ private[client] def newDataset(f: proto.Relation.Builder => Unit): Dataset = {
+ val builder = proto.Relation.newBuilder()
+ f(builder)
+ val plan = proto.Plan.newBuilder().setRoot(builder).build()
+ new Dataset(this, plan)
+ }
+
+ private[client] def analyze(plan: proto.Plan): proto.AnalyzePlanResponse = {
+ val request = proto.AnalyzePlanRequest.newBuilder()
+ .setPlan(plan)
+ .setUserContext(userContext)
+ .build()
+ stub.analyzePlan(request)
+ }
+
+ override def close(): Unit = {
+ channel.shutdownNow()
+ allocator.close()
+ }
+}
+
+object SparkSession {
+ def builder(): Builder = new Builder()
+
+ class Builder() {
+ private val userContextBuilder = proto.UserContext.newBuilder()
+ private var _host: String = "localhost"
+ private var _port: Int = 15002
Review Comment:
Please avoid a magic number. If we cannot use the existing `CONNECT_GRPC_BINDING_PORT`, we may need to move the config definition from `server` to `common` first.
https://github.com/apache/spark/blob/f3be74e3a421224bb65a01e658de96006de2a4ac/connector/connect/server/src/main/scala/org/apache/spark/sql/connect/config/Connect.scala#L24-L28
--
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] zhenlineo commented on a diff in pull request #39078: [SPARK-41534][CONNECT][SQL] Setup initial client module for Spark Connect
Posted by GitBox <gi...@apache.org>.
zhenlineo commented on code in PR #39078:
URL: https://github.com/apache/spark/pull/39078#discussion_r1051273721
##########
project/SparkBuild.scala:
##########
@@ -1433,7 +1492,10 @@ object CopyDependencies {
if (jar.getName.contains("spark-connect") &&
!SbtPomKeys.profiles.value.contains("noshade-connect")) {
Files.copy(fid.toPath, destJar.toPath)
- } else if (jar.getName.contains("spark-protobuf") &&
+ } else if (jar.getName.contains("connect-client") &&
+ !SbtPomKeys.profiles.value.contains("noshade-protobuf")) {
Review Comment:
Just double check if you really meant to use `spark-protobuf` or it is a copy-past typo?
The values to these three jars are a bit magical.
--
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