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 2021/11/20 05:14:26 UTC
[GitHub] [spark] dchvn opened a new pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
dchvn opened a new pull request #34673:
URL: https://github.com/apache/spark/pull/34673
### What changes were proposed in this pull request?
Implementing `createIndex`/`IndexExists`/`dropIndex` in DS V2 JDBC for Postgres dialect.
### Why are the changes needed?
This is a subtask of the V2 Index support. This PR implements `createIndex`, `IndexExists` and `dropIndex`. After review for some changes in this PR, I will create new PR for `listIndexs`, or add it in this PR.
This PR only implements `createIndex`, `IndexExists` and `dropIndex` in Postgres dialect.
### Does this PR introduce _any_ user-facing change?
Yes, `createIndex`/`IndexExists`/`dropIndex` in DS V2 JDBC
### How was this patch tested?
New 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] SparkQA commented on pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974599139
--
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] dchvn commented on a change in pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r753636069
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -193,6 +193,8 @@ private[v2] trait V2JDBCTest extends SharedSparkSession with DockerIntegrationFu
test("SPARK-36895: Test INDEX Using SQL") {
if (supportsIndex) {
+ val indexOptions = if (catalogName.equals("mysql")) "KEY_BLOCK_SIZE=10"
+ else if (catalogName.equals("postgresql")) "FILLFACTOR=70"
Review comment:
Depend on type of jdbc dialect, we change the index options for 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] AmplabJenkins commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-982495103
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50231/
--
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 removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983455444
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50275/
--
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 removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-987620470
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50449/
--
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] cloud-fan commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r767472919
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -1073,6 +1076,81 @@ object JdbcUtils extends Logging with SQLConfHelper {
}
}
+ /**
+ * Check if index exists in a table
+ */
+ def checkIfIndexExists(
+ conn: Connection,
+ sql: String,
+ options: JDBCOptions): Boolean = {
+ val statement = conn.createStatement
+ try {
+ statement.setQueryTimeout(options.queryTimeout)
+ val rs = statement.executeQuery(sql)
+ rs.next
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
+ } finally {
+ statement.close()
+ }
+ }
+
+ /**
+ * Process index properties and return tuple of indexType and list of the other index properties.
+ */
+ def processIndexProperties(
+ properties: util.Map[String, String],
+ catalogName: String
+ ): (String, Array[String]) = {
+ var indexType = ""
+ val indexPropertyList: ArrayBuffer[String] = ArrayBuffer[String]()
+ val supportedIndexTypeList = getSupportedIndexTypeList(catalogName)
+
+ if (!properties.isEmpty) {
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (containsIndexTypeIgnoreCase(supportedIndexTypeList, v)) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ s" The supported Index Types are: ${supportedIndexTypeList.mkString(" AND ")}")
+ }
+ } else {
+ indexPropertyList.append(convertPropertyPairToString(catalogName, k, v))
+ }
+ }
+ }
+ (indexType, indexPropertyList.toArray)
+ }
+
+ def containsIndexTypeIgnoreCase(supportedIndexTypeList: Array[String], value: String): Boolean = {
+ if (supportedIndexTypeList.isEmpty) {
+ throw new UnsupportedOperationException(s"None of index type is supported.")
+ }
+ for (indexType <- supportedIndexTypeList) {
+ if (value.equalsIgnoreCase(indexType)) return true
+ }
+ false
+ }
+
+ def getSupportedIndexTypeList(catalogName: String): Array[String] = {
+ catalogName match {
+ case "mysql" => Array("BTREE", "HASH")
+ case "postgresql" => Array("BTREE", "HASH", "BRIN")
+ case _ => Array.empty
+ }
+ }
Review comment:
So the only benefit of adding this API in `JdbcDialect` is the share the code of checking unsupported index type earlier, which I don't think is worthwhile.
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-984329538
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50314/
--
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] cloud-fan commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r767473789
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -1073,6 +1075,73 @@ object JdbcUtils extends Logging with SQLConfHelper {
}
}
+ /**
+ * Check if index exists in a table
+ */
+ def checkIfIndexExists(
+ conn: Connection,
+ sql: String,
+ options: JDBCOptions): Boolean = {
+ val statement = conn.createStatement
+ try {
+ statement.setQueryTimeout(options.queryTimeout)
+ val rs = statement.executeQuery(sql)
+ rs.next
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
+ } finally {
+ statement.close()
+ }
+ }
+
+ /**
+ * Process index properties and return tuple of indexType and list of the other index properties.
+ */
+ def processIndexProperties(
+ properties: util.Map[String, String],
+ catalogName: String
+ ): (String, Array[String]) = {
Review comment:
```suggestion
catalogName: String): (String, Array[String]) = {
```
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983363812
**[Test build #145801 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145801/testReport)** for PR 34673 at commit [`2ebf415`](https://github.com/apache/spark/commit/2ebf41501ea8d127ab35f2e2453aa854b8e08366).
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983403563
**[Test build #145806 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145806/testReport)** for PR 34673 at commit [`ac56e68`](https://github.com/apache/spark/commit/ac56e683df18331e1a1fa82d087b51cfc100ecb8).
--
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 removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983545863
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50280/
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983629803
**[Test build #145806 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145806/testReport)** for PR 34673 at commit [`ac56e68`](https://github.com/apache/spark/commit/ac56e683df18331e1a1fa82d087b51cfc100ecb8).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] huaxingao commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r762662920
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -38,6 +40,7 @@ import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
import org.apache.spark.sql.catalyst.util.{CaseInsensitiveMap, DateTimeUtils, GenericArrayData}
import org.apache.spark.sql.catalyst.util.DateTimeUtils.{instantToMicros, localDateToDays, toJavaDate, toJavaTimestamp}
import org.apache.spark.sql.connector.catalog.TableChange
+import org.apache.spark.sql.connector.catalog.index.SupportsIndex
Review comment:
nit: combine this with the next line
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-982438167
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50231/
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-995587616
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50746/
--
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] SparkQA commented on pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974627366
**[Test build #145475 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145475/testReport)** for PR 34673 at commit [`baae935`](https://github.com/apache/spark/commit/baae935fc0c0cc47c4397be662ac7d36dfbbe8b0).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983406703
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50275/
--
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] SparkQA removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983403563
**[Test build #145806 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145806/testReport)** for PR 34673 at commit [`ac56e68`](https://github.com/apache/spark/commit/ac56e683df18331e1a1fa82d087b51cfc100ecb8).
--
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 removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-982631485
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145759/
--
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 #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-982631485
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145759/
--
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] dchvn commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-997509360
Thank you!!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-992129640
Looks good to me except this https://github.com/apache/spark/pull/34673/files#r762664569
cc @cloud-fan
--
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] dchvn commented on pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974602693
CC @huaxingao FYI. Please take a look when you find some time, thanks!
--
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 #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974630135
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145475/
--
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 #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974610792
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49947/
--
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] cloud-fan commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-996686528
thanks, merging to master!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-984331466
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50314/
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-982629411
**[Test build #145759 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145759/testReport)** for PR 34673 at commit [`0e04370`](https://github.com/apache/spark/commit/0e0437041b5f389be3298855e040b5040918177e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] dchvn commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r759855352
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -1078,26 +1080,50 @@ object JdbcUtils extends Logging with SQLConfHelper {
*/
def checkIfIndexExists(
conn: Connection,
- indexName: String,
sql: String,
- indexColumnName: String,
options: JDBCOptions): Boolean = {
val statement = conn.createStatement
try {
statement.setQueryTimeout(options.queryTimeout)
val rs = statement.executeQuery(sql)
- while (rs.next()) {
- val retrievedIndexName = rs.getString(indexColumnName)
- if (conf.resolver(retrievedIndexName, indexName)) {
- return true
- }
- }
- false
+ rs.next
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
} finally {
statement.close()
}
}
+ /**
+ * Process index properties and return tuple of indexType and list of the other index properties.
+ */
+ def processIndexProperties(
+ properties: util.Map[String, String],
+ options: JDBCOptions
+ ): (String, Array[String]) = {
+ val dialect = JdbcDialects.get(options.url)
+ var indexType = ""
+ var indexPropertyList: Array[String] = Array.empty
+
+ if (!properties.isEmpty) {
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (v.equalsIgnoreCase("BTREE") || v.equalsIgnoreCase("HASH")) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ " The supported Index Types are: BTREE and HASH")
Review comment:
@huaxingao, `GIST` and `GIN` index types in PostgreSQL are used for specific types: `tsvector` and `tsquery`, which are not supported types in Spark. Also for `SP-GIST` are used for the `point` type. Should we ignore these index types or something else? [GIST and GIN index type](https://www.postgresql.org/docs/9.1/textsearch-indexes.html) for details.
--
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] SparkQA removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983363812
**[Test build #145801 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145801/testReport)** for PR 34673 at commit [`2ebf415`](https://github.com/apache/spark/commit/2ebf41501ea8d127ab35f2e2453aa854b8e08366).
--
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] dchvn commented on a change in pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r753640549
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala
##########
@@ -164,4 +172,78 @@ private object PostgresDialect extends JdbcDialect {
s"TABLESAMPLE BERNOULLI" +
s" (${(sample.upperBound - sample.lowerBound) * 100}) REPEATABLE (${sample.seed})"
}
+
+ // CREATE INDEX syntax
+ // https://www.postgresql.org/docs/14/sql-createindex.html
+ override def createIndex(
+ indexName: String,
+ tableName: String,
+ columns: Array[NamedReference],
+ columnsProperties: util.Map[NamedReference, util.Map[String, String]],
+ properties: util.Map[String, String]): String = {
+ val columnList = columns.map(col => quoteIdentifier(col.fieldNames.head))
+ var indexPropertiesStr: String = ""
+ var hasIndexProperties: Boolean = false
+ var indexType = ""
+
+ if (!properties.isEmpty) {
+ var indexPropertyList: Array[String] = Array.empty
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (v.equalsIgnoreCase("BTREE") || v.equalsIgnoreCase("HASH")) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ " The supported Index Types are: BTREE and HASH")
+ }
+ } else {
+ hasIndexProperties = true
+ indexPropertyList = indexPropertyList :+ s"$k = $v"
+ }
+ }
+ if (hasIndexProperties) {
+ indexPropertiesStr += "WITH (" + indexPropertyList.mkString(", ") + ")"
+ }
+ }
+
+ s"CREATE INDEX ${quoteIdentifier(indexName)} ON ${quoteIdentifier(tableName)}" +
+ s" $indexType (${columnList.mkString(", ")}) $indexPropertiesStr"
+ }
+
+ // SHOW INDEX syntax
+ // https://www.postgresql.org/docs/14/view-pg-indexes.html
+ override def indexExists(
+ conn: Connection,
+ indexName: String,
+ tableName: String,
+ options: JDBCOptions): Boolean = {
+ val sql = s"SELECT * FROM pg_indexes WHERE tablename = '$tableName'"
+ try {
+ JdbcUtils.checkIfIndexExists(conn, indexName, sql, "indexname", options)
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
+ }
+ }
+
+ // DROP INDEX syntax
+ // https://www.postgresql.org/docs/14/sql-dropindex.html
+ override def dropIndex(indexName: String, tableName: String): String = {
+ s"DROP INDEX ${quoteIdentifier(indexName)}"
+ }
+
+ override def classifyException(message: String, e: Throwable): AnalysisException = {
+ e match {
+ case sqlException: SQLException =>
+ sqlException.getSQLState match {
+ // https://www.postgresql.org/docs/14/errcodes-appendix.html
+ case "42P07" => throw new IndexAlreadyExistsException(message, cause = Some(e))
+ case "42704" => throw new NoSuchIndexException(message, cause = Some(e))
Review comment:
I use Postgres error codes to handle the exception, but `getErrorCode` returns `0`. So I use `getSQLState` instead.
--
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] SparkQA removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-982394534
**[Test build #145759 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145759/testReport)** for PR 34673 at commit [`0e04370`](https://github.com/apache/spark/commit/0e0437041b5f389be3298855e040b5040918177e).
--
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] cloud-fan closed pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #34673:
URL: https://github.com/apache/spark/pull/34673
--
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] dchvn commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r763604274
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -1073,6 +1076,81 @@ object JdbcUtils extends Logging with SQLConfHelper {
}
}
+ /**
+ * Check if index exists in a table
+ */
+ def checkIfIndexExists(
+ conn: Connection,
+ sql: String,
+ options: JDBCOptions): Boolean = {
+ val statement = conn.createStatement
+ try {
+ statement.setQueryTimeout(options.queryTimeout)
+ val rs = statement.executeQuery(sql)
+ rs.next
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
+ } finally {
+ statement.close()
+ }
+ }
+
+ /**
+ * Process index properties and return tuple of indexType and list of the other index properties.
+ */
+ def processIndexProperties(
+ properties: util.Map[String, String],
+ catalogName: String
+ ): (String, Array[String]) = {
+ var indexType = ""
+ val indexPropertyList: ArrayBuffer[String] = ArrayBuffer[String]()
+ val supportedIndexTypeList = getSupportedIndexTypeList(catalogName)
+
+ if (!properties.isEmpty) {
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (containsIndexTypeIgnoreCase(supportedIndexTypeList, v)) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ s" The supported Index Types are: ${supportedIndexTypeList.mkString(" AND ")}")
+ }
+ } else {
+ indexPropertyList.append(convertPropertyPairToString(catalogName, k, v))
+ }
+ }
+ }
+ (indexType, indexPropertyList.toArray)
+ }
+
+ def containsIndexTypeIgnoreCase(supportedIndexTypeList: Array[String], value: String): Boolean = {
+ if (supportedIndexTypeList.isEmpty) {
+ throw new UnsupportedOperationException(s"None of index type is supported.")
+ }
+ for (indexType <- supportedIndexTypeList) {
+ if (value.equalsIgnoreCase(indexType)) return true
+ }
+ false
+ }
+
+ def getSupportedIndexTypeList(catalogName: String): Array[String] = {
+ catalogName match {
+ case "mysql" => Array("BTREE", "HASH")
+ case "postgresql" => Array("BTREE", "HASH", "BRIN")
+ case _ => Array.empty
+ }
+ }
Review comment:
Yeah, I agree with @huaxingao. Should we move it to `JdbcDialect` @cloud-fan?
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-987565780
**[Test build #145974 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145974/testReport)** for PR 34673 at commit [`67338d4`](https://github.com/apache/spark/commit/67338d476bd340f436e7ad7cb4020375a7230f0e).
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-984302447
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50314/
--
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 #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-987748927
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145974/
--
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] cloud-fan commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r767474830
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -1073,6 +1075,73 @@ object JdbcUtils extends Logging with SQLConfHelper {
}
}
+ /**
+ * Check if index exists in a table
+ */
+ def checkIfIndexExists(
+ conn: Connection,
+ sql: String,
+ options: JDBCOptions): Boolean = {
+ val statement = conn.createStatement
+ try {
+ statement.setQueryTimeout(options.queryTimeout)
+ val rs = statement.executeQuery(sql)
+ rs.next
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
+ } finally {
+ statement.close()
+ }
+ }
+
+ /**
+ * Process index properties and return tuple of indexType and list of the other index properties.
+ */
+ def processIndexProperties(
+ properties: util.Map[String, String],
+ catalogName: String
+ ): (String, Array[String]) = {
+ var indexType = ""
+ val indexPropertyList: ArrayBuffer[String] = ArrayBuffer[String]()
+ val supportedIndexTypeList = getSupportedIndexTypeList(catalogName)
+
+ if (!properties.isEmpty) {
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (containsIndexTypeIgnoreCase(supportedIndexTypeList, v)) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ s" The supported Index Types are: ${supportedIndexTypeList.mkString(" AND ")}")
+ }
+ } else {
+ indexPropertyList.append(s"$k = $v")
+ }
+ }
+ }
+ (indexType, indexPropertyList.toArray)
+ }
+
+ def containsIndexTypeIgnoreCase(supportedIndexTypeList: Array[String], value: String): Boolean = {
+ if (supportedIndexTypeList.isEmpty) {
+ throw new UnsupportedOperationException(s"None of index type is supported.")
Review comment:
to be more user-facing: `Cannot specify 'USING index_type' in 'CREATE INDEX'`
--
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] dchvn commented on a change in pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r753640303
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala
##########
@@ -164,4 +172,78 @@ private object PostgresDialect extends JdbcDialect {
s"TABLESAMPLE BERNOULLI" +
s" (${(sample.upperBound - sample.lowerBound) * 100}) REPEATABLE (${sample.seed})"
}
+
+ // CREATE INDEX syntax
+ // https://www.postgresql.org/docs/14/sql-createindex.html
+ override def createIndex(
+ indexName: String,
+ tableName: String,
+ columns: Array[NamedReference],
+ columnsProperties: util.Map[NamedReference, util.Map[String, String]],
+ properties: util.Map[String, String]): String = {
+ val columnList = columns.map(col => quoteIdentifier(col.fieldNames.head))
+ var indexPropertiesStr: String = ""
+ var hasIndexProperties: Boolean = false
+ var indexType = ""
+
+ if (!properties.isEmpty) {
+ var indexPropertyList: Array[String] = Array.empty
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (v.equalsIgnoreCase("BTREE") || v.equalsIgnoreCase("HASH")) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ " The supported Index Types are: BTREE and HASH")
+ }
+ } else {
+ hasIndexProperties = true
+ indexPropertyList = indexPropertyList :+ s"$k = $v"
+ }
+ }
+ if (hasIndexProperties) {
+ indexPropertiesStr += "WITH (" + indexPropertyList.mkString(", ") + ")"
+ }
+ }
+
+ s"CREATE INDEX ${quoteIdentifier(indexName)} ON ${quoteIdentifier(tableName)}" +
+ s" $indexType (${columnList.mkString(", ")}) $indexPropertiesStr"
+ }
+
+ // SHOW INDEX syntax
+ // https://www.postgresql.org/docs/14/view-pg-indexes.html
+ override def indexExists(
+ conn: Connection,
+ indexName: String,
+ tableName: String,
+ options: JDBCOptions): Boolean = {
+ val sql = s"SELECT * FROM pg_indexes WHERE tablename = '$tableName'"
+ try {
+ JdbcUtils.checkIfIndexExists(conn, indexName, sql, "indexname", options)
Review comment:
I can not use `JdbcUtils.executeQuery(conn, options, sql)`. When I use it to execute query show index in Postgres, `rs.next()` throw exception `"The ResultSet is closed"`. That function return `resultset`, but it is closed when `statement` is closed in finally block. As in java 8 docs: [ResultSet](https://docs.oracle.com/javase/8/docs/api/java/sql/ResultSet.html#:~:text=A%20ResultSet%20object%20is%20automatically%20closed%20when%20the%20Statement%20object%20that%20generated%20it%20is%20closed%2C%20re-executed%2C%20or%20used%20to%20retrieve%20the%20next%20result%20from%20a%20sequence%20of%20multiple%20results.).
So I create `JdbcUtils.checkIfIndexExists` to execute the resultset in `JdbcUtils` for checking if the index exists, and just return boolean. WDYT, @huaxingao?
--
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] dchvn commented on pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974602693
CC @huaxingao FYI. Please take a look when you find some time, thanks!
--
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 removed a comment on pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974610792
--
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 removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-995589010
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50746/
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-995801755
**[Test build #146271 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146271/testReport)** for PR 34673 at commit [`36ee897`](https://github.com/apache/spark/commit/36ee897e7a12e4631f75d6d8cb057bd922c71437).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] huaxingao commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r758025776
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala
##########
@@ -164,4 +172,78 @@ private object PostgresDialect extends JdbcDialect {
s"TABLESAMPLE BERNOULLI" +
s" (${(sample.upperBound - sample.lowerBound) * 100}) REPEATABLE (${sample.seed})"
}
+
+ // CREATE INDEX syntax
+ // https://www.postgresql.org/docs/14/sql-createindex.html
+ override def createIndex(
+ indexName: String,
+ tableName: String,
+ columns: Array[NamedReference],
+ columnsProperties: util.Map[NamedReference, util.Map[String, String]],
+ properties: util.Map[String, String]): String = {
+ val columnList = columns.map(col => quoteIdentifier(col.fieldNames.head))
+ var indexPropertiesStr: String = ""
+ var hasIndexProperties: Boolean = false
+ var indexType = ""
+
+ if (!properties.isEmpty) {
+ var indexPropertyList: Array[String] = Array.empty
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (v.equalsIgnoreCase("BTREE") || v.equalsIgnoreCase("HASH")) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ " The supported Index Types are: BTREE and HASH")
+ }
+ } else {
+ hasIndexProperties = true
+ indexPropertyList = indexPropertyList :+ s"$k = $v"
+ }
+ }
+ if (hasIndexProperties) {
+ indexPropertiesStr += "WITH (" + indexPropertyList.mkString(", ") + ")"
+ }
+ }
+
+ s"CREATE INDEX ${quoteIdentifier(indexName)} ON ${quoteIdentifier(tableName)}" +
+ s" $indexType (${columnList.mkString(", ")}) $indexPropertiesStr"
+ }
+
+ // SHOW INDEX syntax
+ // https://www.postgresql.org/docs/14/view-pg-indexes.html
+ override def indexExists(
+ conn: Connection,
+ indexName: String,
+ tableName: String,
+ options: JDBCOptions): Boolean = {
+ val sql = s"SELECT * FROM pg_indexes WHERE tablename = '$tableName'"
+ try {
+ JdbcUtils.checkIfIndexExists(conn, indexName, sql, "indexname", options)
Review comment:
How about moving the try catch to `JdbcUtils` too? So here we have
```
val sql = s"SELECT * FROM pg_indexes WHERE tablename = ${quoteIdentifier(tableName)} AND" +
s" indexname = '$indexName'"
JdbcUtils.checkIfIndexExists(conn, sql, options)
```
In `JDBCUtils`, we have
```
def checkIfIndexExists(
conn: Connection,
sql: String,
options: JDBCOptions): Boolean = {
val statement = conn.createStatement
try {
statement.setQueryTimeout(options.queryTimeout)
val rs = statement.executeQuery(sql)
rs.next
} catch {
case _: Exception =>
logWarning("Cannot retrieved index info.")
false
} finally {
statement.close()
}
}
```
--
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] cloud-fan commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r759950310
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -193,6 +193,8 @@ private[v2] trait V2JDBCTest extends SharedSparkSession with DockerIntegrationFu
test("SPARK-36895: Test INDEX Using SQL") {
if (supportsIndex) {
+ val indexOptions = if (catalogName.equals("mysql")) "KEY_BLOCK_SIZE=10"
+ else if (catalogName.equals("postgresql")) "FILLFACTOR=70"
Review comment:
Can we add a method for this and implement it in each concrete test suite? e.g. `def indexOptions: String = ""`
--
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 #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983639467
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145806/
--
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] dchvn commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-984286980
@huaxingao @cloud-fan Updated! Please take a look if you have time. Many thanks.
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983455409
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50275/
--
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 removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983599243
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145801/
--
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] cloud-fan commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r767472272
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -1073,6 +1076,81 @@ object JdbcUtils extends Logging with SQLConfHelper {
}
}
+ /**
+ * Check if index exists in a table
+ */
+ def checkIfIndexExists(
+ conn: Connection,
+ sql: String,
+ options: JDBCOptions): Boolean = {
+ val statement = conn.createStatement
+ try {
+ statement.setQueryTimeout(options.queryTimeout)
+ val rs = statement.executeQuery(sql)
+ rs.next
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
+ } finally {
+ statement.close()
+ }
+ }
+
+ /**
+ * Process index properties and return tuple of indexType and list of the other index properties.
+ */
+ def processIndexProperties(
+ properties: util.Map[String, String],
+ catalogName: String
+ ): (String, Array[String]) = {
+ var indexType = ""
+ val indexPropertyList: ArrayBuffer[String] = ArrayBuffer[String]()
+ val supportedIndexTypeList = getSupportedIndexTypeList(catalogName)
+
+ if (!properties.isEmpty) {
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (containsIndexTypeIgnoreCase(supportedIndexTypeList, v)) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ s" The supported Index Types are: ${supportedIndexTypeList.mkString(" AND ")}")
+ }
+ } else {
+ indexPropertyList.append(convertPropertyPairToString(catalogName, k, v))
+ }
+ }
+ }
+ (indexType, indexPropertyList.toArray)
+ }
+
+ def containsIndexTypeIgnoreCase(supportedIndexTypeList: Array[String], value: String): Boolean = {
+ if (supportedIndexTypeList.isEmpty) {
+ throw new UnsupportedOperationException(s"None of index type is supported.")
+ }
+ for (indexType <- supportedIndexTypeList) {
+ if (value.equalsIgnoreCase(indexType)) return true
+ }
+ false
+ }
+
+ def getSupportedIndexTypeList(catalogName: String): Array[String] = {
+ catalogName match {
+ case "mysql" => Array("BTREE", "HASH")
+ case "postgresql" => Array("BTREE", "HASH", "BRIN")
+ case _ => Array.empty
+ }
+ }
Review comment:
I don't think so. This is for failing earlier in `createIndex`, where the JDBC dialect implementation already has full control.
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-995547503
**[Test build #146271 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146271/testReport)** for PR 34673 at commit [`36ee897`](https://github.com/apache/spark/commit/36ee897e7a12e4631f75d6d8cb057bd922c71437).
--
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] dchvn commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r758113045
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala
##########
@@ -164,4 +172,78 @@ private object PostgresDialect extends JdbcDialect {
s"TABLESAMPLE BERNOULLI" +
s" (${(sample.upperBound - sample.lowerBound) * 100}) REPEATABLE (${sample.seed})"
}
+
+ // CREATE INDEX syntax
+ // https://www.postgresql.org/docs/14/sql-createindex.html
+ override def createIndex(
+ indexName: String,
+ tableName: String,
+ columns: Array[NamedReference],
+ columnsProperties: util.Map[NamedReference, util.Map[String, String]],
+ properties: util.Map[String, String]): String = {
Review comment:
@huaxingao The implementation is not totally similar to `MySQL` in the assignment of variables. So we should create a function with the type of JDBC parameter that we base on it to return the exact SQL, right? Thanks!
--
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 removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983639467
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145806/
--
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 #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983545863
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50280/
--
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 #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974610792
--
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] SparkQA commented on pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974599139
**[Test build #145475 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145475/testReport)** for PR 34673 at commit [`baae935`](https://github.com/apache/spark/commit/baae935fc0c0cc47c4397be662ac7d36dfbbe8b0).
--
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] dchvn commented on a change in pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r753636069
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -193,6 +193,8 @@ private[v2] trait V2JDBCTest extends SharedSparkSession with DockerIntegrationFu
test("SPARK-36895: Test INDEX Using SQL") {
if (supportsIndex) {
+ val indexOptions = if (catalogName.equals("mysql")) "KEY_BLOCK_SIZE=10"
+ else if (catalogName.equals("postgresql")) "FILLFACTOR=70"
Review comment:
Depend on type of jdbc dialect, we change the index options for test.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala
##########
@@ -164,4 +172,78 @@ private object PostgresDialect extends JdbcDialect {
s"TABLESAMPLE BERNOULLI" +
s" (${(sample.upperBound - sample.lowerBound) * 100}) REPEATABLE (${sample.seed})"
}
+
+ // CREATE INDEX syntax
+ // https://www.postgresql.org/docs/14/sql-createindex.html
+ override def createIndex(
+ indexName: String,
+ tableName: String,
+ columns: Array[NamedReference],
+ columnsProperties: util.Map[NamedReference, util.Map[String, String]],
+ properties: util.Map[String, String]): String = {
+ val columnList = columns.map(col => quoteIdentifier(col.fieldNames.head))
+ var indexPropertiesStr: String = ""
+ var hasIndexProperties: Boolean = false
+ var indexType = ""
+
+ if (!properties.isEmpty) {
+ var indexPropertyList: Array[String] = Array.empty
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (v.equalsIgnoreCase("BTREE") || v.equalsIgnoreCase("HASH")) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ " The supported Index Types are: BTREE and HASH")
+ }
+ } else {
+ hasIndexProperties = true
+ indexPropertyList = indexPropertyList :+ s"$k = $v"
+ }
+ }
+ if (hasIndexProperties) {
+ indexPropertiesStr += "WITH (" + indexPropertyList.mkString(", ") + ")"
+ }
+ }
+
+ s"CREATE INDEX ${quoteIdentifier(indexName)} ON ${quoteIdentifier(tableName)}" +
+ s" $indexType (${columnList.mkString(", ")}) $indexPropertiesStr"
+ }
+
+ // SHOW INDEX syntax
+ // https://www.postgresql.org/docs/14/view-pg-indexes.html
+ override def indexExists(
+ conn: Connection,
+ indexName: String,
+ tableName: String,
+ options: JDBCOptions): Boolean = {
+ val sql = s"SELECT * FROM pg_indexes WHERE tablename = '$tableName'"
+ try {
+ JdbcUtils.checkIfIndexExists(conn, indexName, sql, "indexname", options)
Review comment:
I can not use `JdbcUtils.executeQuery(conn, options, sql)`. When I use it to execute query show index in Postgres, `rs.next()` throw exception `"The ResultSet is closed"`. That function return `resultset`, but it is closed when `statement` is closed in finally block. As in java 8 docs: [ResultSet](https://docs.oracle.com/javase/8/docs/api/java/sql/ResultSet.html#:~:text=A%20ResultSet%20object%20is%20automatically%20closed%20when%20the%20Statement%20object%20that%20generated%20it%20is%20closed%2C%20re-executed%2C%20or%20used%20to%20retrieve%20the%20next%20result%20from%20a%20sequence%20of%20multiple%20results.).
So I create `JdbcUtils.checkIfIndexExists` to execute the resultset in `JdbcUtils` for checking if the index exists, and just return boolean. WDYT, @huaxingao?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala
##########
@@ -164,4 +172,78 @@ private object PostgresDialect extends JdbcDialect {
s"TABLESAMPLE BERNOULLI" +
s" (${(sample.upperBound - sample.lowerBound) * 100}) REPEATABLE (${sample.seed})"
}
+
+ // CREATE INDEX syntax
+ // https://www.postgresql.org/docs/14/sql-createindex.html
+ override def createIndex(
+ indexName: String,
+ tableName: String,
+ columns: Array[NamedReference],
+ columnsProperties: util.Map[NamedReference, util.Map[String, String]],
+ properties: util.Map[String, String]): String = {
+ val columnList = columns.map(col => quoteIdentifier(col.fieldNames.head))
+ var indexPropertiesStr: String = ""
+ var hasIndexProperties: Boolean = false
+ var indexType = ""
+
+ if (!properties.isEmpty) {
+ var indexPropertyList: Array[String] = Array.empty
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (v.equalsIgnoreCase("BTREE") || v.equalsIgnoreCase("HASH")) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ " The supported Index Types are: BTREE and HASH")
+ }
+ } else {
+ hasIndexProperties = true
+ indexPropertyList = indexPropertyList :+ s"$k = $v"
+ }
+ }
+ if (hasIndexProperties) {
+ indexPropertiesStr += "WITH (" + indexPropertyList.mkString(", ") + ")"
+ }
+ }
+
+ s"CREATE INDEX ${quoteIdentifier(indexName)} ON ${quoteIdentifier(tableName)}" +
+ s" $indexType (${columnList.mkString(", ")}) $indexPropertiesStr"
+ }
+
+ // SHOW INDEX syntax
+ // https://www.postgresql.org/docs/14/view-pg-indexes.html
+ override def indexExists(
+ conn: Connection,
+ indexName: String,
+ tableName: String,
+ options: JDBCOptions): Boolean = {
+ val sql = s"SELECT * FROM pg_indexes WHERE tablename = '$tableName'"
+ try {
+ JdbcUtils.checkIfIndexExists(conn, indexName, sql, "indexname", options)
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
+ }
+ }
+
+ // DROP INDEX syntax
+ // https://www.postgresql.org/docs/14/sql-dropindex.html
+ override def dropIndex(indexName: String, tableName: String): String = {
+ s"DROP INDEX ${quoteIdentifier(indexName)}"
+ }
+
+ override def classifyException(message: String, e: Throwable): AnalysisException = {
+ e match {
+ case sqlException: SQLException =>
+ sqlException.getSQLState match {
+ // https://www.postgresql.org/docs/14/errcodes-appendix.html
+ case "42P07" => throw new IndexAlreadyExistsException(message, cause = Some(e))
+ case "42704" => throw new NoSuchIndexException(message, cause = Some(e))
Review comment:
I use Postgres error codes to handle the exception, but `getErrorCode` returns `0`. So I use `getSQLState` instead.
--
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] SparkQA removed a comment on pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974599139
**[Test build #145475 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145475/testReport)** for PR 34673 at commit [`baae935`](https://github.com/apache/spark/commit/baae935fc0c0cc47c4397be662ac7d36dfbbe8b0).
--
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] SparkQA removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-987565780
**[Test build #145974 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145974/testReport)** for PR 34673 at commit [`67338d4`](https://github.com/apache/spark/commit/67338d476bd340f436e7ad7cb4020375a7230f0e).
--
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] SparkQA commented on pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974609349
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49947/
--
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] huaxingao commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r762663958
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -1073,6 +1076,81 @@ object JdbcUtils extends Logging with SQLConfHelper {
}
}
+ /**
+ * Check if index exists in a table
+ */
+ def checkIfIndexExists(
+ conn: Connection,
+ sql: String,
+ options: JDBCOptions): Boolean = {
+ val statement = conn.createStatement
+ try {
+ statement.setQueryTimeout(options.queryTimeout)
+ val rs = statement.executeQuery(sql)
+ rs.next
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
+ } finally {
+ statement.close()
+ }
+ }
+
+ /**
+ * Process index properties and return tuple of indexType and list of the other index properties.
+ */
+ def processIndexProperties(
+ properties: util.Map[String, String],
+ catalogName: String
+ ): (String, Array[String]) = {
+ var indexType = ""
+ val indexPropertyList: ArrayBuffer[String] = ArrayBuffer[String]()
+ val supportedIndexTypeList = getSupportedIndexTypeList(catalogName)
+
+ if (!properties.isEmpty) {
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (containsIndexTypeIgnoreCase(supportedIndexTypeList, v)) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ s" The supported Index Types are: ${supportedIndexTypeList.mkString(" AND ")}")
+ }
+ } else {
+ indexPropertyList.append(convertPropertyPairToString(catalogName, k, v))
+ }
+ }
+ }
+ (indexType, indexPropertyList.toArray)
+ }
+
+ def containsIndexTypeIgnoreCase(supportedIndexTypeList: Array[String], value: String): Boolean = {
+ if (supportedIndexTypeList.isEmpty) {
+ throw new UnsupportedOperationException(s"None of index type is supported.")
+ }
+ for (indexType <- supportedIndexTypeList) {
+ if (value.equalsIgnoreCase(indexType)) return true
+ }
+ false
+ }
+
+ def getSupportedIndexTypeList(catalogName: String): Array[String] = {
+ catalogName match {
+ case "mysql" => Array("BTREE", "HASH")
+ case "postgresql" => Array("BTREE", "HASH", "BRIN")
+ case _ => Array.empty
+ }
+ }
+
+ def convertPropertyPairToString(catalogName: String, key: String, value: String): String = {
+ catalogName match {
+ case "mysql" => s"$key $value"
+ case "postgresql" => s"$key = $value"
+ case _ => ""
+ }
+ }
Review comment:
I think we don't need this method for now. Seems to me that databases always allow this `=` in between of key and value. I didn't verify this syntax in all the databases, but I checked MySQL, it takes `=` as optional.
--
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] huaxingao commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r762662738
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -71,6 +71,14 @@ private[v2] trait V2JDBCTest extends SharedSparkSession with DockerIntegrationFu
def testCreateTableWithProperty(tbl: String): Unit = {}
+ def indexOptions(catalogName: String): String = {
+ catalogName match {
+ case "mysql" => "KEY_BLOCK_SIZE=10"
+ case "postgresql" => "FILLFACTOR=70"
+ case _ => ""
+ }
+ }
Review comment:
I think what @cloud-fan meant is to add a method such as
```
def indexOptions: String = ""
```
then in each of the concrete test suite, implement this method, e.g. in `MySQLIntegrationSuite`, implement it as
```
override def indexOptions: String = "KEY_BLOCK_SIZE=10"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-995813458
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146271/
--
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] dchvn commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-982398696
CC @huaxingao. Updated. Can you re-review it? Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-982479155
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50231/
--
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 removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-982495103
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50231/
--
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] cloud-fan commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r759951994
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -1073,6 +1075,66 @@ object JdbcUtils extends Logging with SQLConfHelper {
}
}
+ /**
+ * Check if index exists in a table
+ */
+ def checkIfIndexExists(
+ conn: Connection,
+ sql: String,
+ options: JDBCOptions): Boolean = {
+ val statement = conn.createStatement
+ try {
+ statement.setQueryTimeout(options.queryTimeout)
+ val rs = statement.executeQuery(sql)
+ rs.next
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
+ } finally {
+ statement.close()
+ }
+ }
+
+ /**
+ * Process index properties and return tuple of indexType and list of the other index properties.
+ */
+ def processIndexProperties(
+ properties: util.Map[String, String],
+ options: JDBCOptions
+ ): (String, Array[String]) = {
+ val dialect = JdbcDialects.get(options.url)
+ var indexType = ""
+ var indexPropertyList: Array[String] = Array.empty
Review comment:
this should be ArrayBuffer
--
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 #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-984286791
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145839/
--
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] huaxingao commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r759743604
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -1078,26 +1080,50 @@ object JdbcUtils extends Logging with SQLConfHelper {
*/
def checkIfIndexExists(
conn: Connection,
- indexName: String,
sql: String,
- indexColumnName: String,
options: JDBCOptions): Boolean = {
val statement = conn.createStatement
try {
statement.setQueryTimeout(options.queryTimeout)
val rs = statement.executeQuery(sql)
- while (rs.next()) {
- val retrievedIndexName = rs.getString(indexColumnName)
- if (conf.resolver(retrievedIndexName, indexName)) {
- return true
- }
- }
- false
+ rs.next
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
} finally {
statement.close()
}
}
+ /**
+ * Process index properties and return tuple of indexType and list of the other index properties.
+ */
+ def processIndexProperties(
+ properties: util.Map[String, String],
+ options: JDBCOptions
+ ): (String, Array[String]) = {
+ val dialect = JdbcDialects.get(options.url)
+ var indexType = ""
+ var indexPropertyList: Array[String] = Array.empty
+
+ if (!properties.isEmpty) {
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (v.equalsIgnoreCase("BTREE") || v.equalsIgnoreCase("HASH")) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ " The supported Index Types are: BTREE and HASH")
Review comment:
Seems PostgreSQL can support more index types (https://www.postgresql.org/docs/9.5/sql-createindex.html).
--
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 #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983599243
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145801/
--
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 removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-984331466
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50314/
--
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] dchvn commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-981259776
cc @huaxingao, @cloud-fan, @viirya FYI. Can you pls take a look? Thanks!
--
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] SparkQA commented on pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974603469
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49947/
--
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 #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-995589010
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50746/
--
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] cloud-fan commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r767474830
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -1073,6 +1075,73 @@ object JdbcUtils extends Logging with SQLConfHelper {
}
}
+ /**
+ * Check if index exists in a table
+ */
+ def checkIfIndexExists(
+ conn: Connection,
+ sql: String,
+ options: JDBCOptions): Boolean = {
+ val statement = conn.createStatement
+ try {
+ statement.setQueryTimeout(options.queryTimeout)
+ val rs = statement.executeQuery(sql)
+ rs.next
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
+ } finally {
+ statement.close()
+ }
+ }
+
+ /**
+ * Process index properties and return tuple of indexType and list of the other index properties.
+ */
+ def processIndexProperties(
+ properties: util.Map[String, String],
+ catalogName: String
+ ): (String, Array[String]) = {
+ var indexType = ""
+ val indexPropertyList: ArrayBuffer[String] = ArrayBuffer[String]()
+ val supportedIndexTypeList = getSupportedIndexTypeList(catalogName)
+
+ if (!properties.isEmpty) {
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (containsIndexTypeIgnoreCase(supportedIndexTypeList, v)) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ s" The supported Index Types are: ${supportedIndexTypeList.mkString(" AND ")}")
+ }
+ } else {
+ indexPropertyList.append(s"$k = $v")
+ }
+ }
+ }
+ (indexType, indexPropertyList.toArray)
+ }
+
+ def containsIndexTypeIgnoreCase(supportedIndexTypeList: Array[String], value: String): Boolean = {
+ if (supportedIndexTypeList.isEmpty) {
+ throw new UnsupportedOperationException(s"None of index type is supported.")
Review comment:
to be more user-facing: `Cannot specify index type in CREATE INDEX`
--
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 removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-984286791
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145839/
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-982394534
**[Test build #145759 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145759/testReport)** for PR 34673 at commit [`0e04370`](https://github.com/apache/spark/commit/0e0437041b5f389be3298855e040b5040918177e).
--
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 #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983455444
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50275/
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983523358
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50280/
--
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] huaxingao commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r758026935
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala
##########
@@ -164,4 +172,78 @@ private object PostgresDialect extends JdbcDialect {
s"TABLESAMPLE BERNOULLI" +
s" (${(sample.upperBound - sample.lowerBound) * 100}) REPEATABLE (${sample.seed})"
}
+
+ // CREATE INDEX syntax
+ // https://www.postgresql.org/docs/14/sql-createindex.html
+ override def createIndex(
+ indexName: String,
+ tableName: String,
+ columns: Array[NamedReference],
+ columnsProperties: util.Map[NamedReference, util.Map[String, String]],
+ properties: util.Map[String, String]): String = {
Review comment:
The implementation is similar to `MySQL`. We probably want to move this to `JdbcUtils` too for code reuse.
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983597809
**[Test build #145801 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145801/testReport)** for PR 34673 at commit [`2ebf415`](https://github.com/apache/spark/commit/2ebf41501ea8d127ab35f2e2453aa854b8e08366).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-987587145
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50449/
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-987618020
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50449/
--
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 removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-987748927
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145974/
--
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] SparkQA removed a comment on pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974599139
**[Test build #145475 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145475/testReport)** for PR 34673 at commit [`baae935`](https://github.com/apache/spark/commit/baae935fc0c0cc47c4397be662ac7d36dfbbe8b0).
--
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] cloud-fan commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r759956337
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala
##########
@@ -358,6 +359,18 @@ abstract class JdbcDialect extends Serializable with Logging{
new AnalysisException(message, cause = Some(e))
}
+ /**
+ * Convert key-value property pair to string
+ */
+ def convertPropertyPairToString(key: String, value: String): String = {
Review comment:
Note that, every method in this class is a public API, and we should be very careful when adding new public APIs.
I don't think the two newly added APIs are necessary. They are just used to share the code between different dialects. We should use internal util functions to share code.
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-983489905
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50280/
--
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] huaxingao commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r762664569
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -1073,6 +1076,81 @@ object JdbcUtils extends Logging with SQLConfHelper {
}
}
+ /**
+ * Check if index exists in a table
+ */
+ def checkIfIndexExists(
+ conn: Connection,
+ sql: String,
+ options: JDBCOptions): Boolean = {
+ val statement = conn.createStatement
+ try {
+ statement.setQueryTimeout(options.queryTimeout)
+ val rs = statement.executeQuery(sql)
+ rs.next
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
+ } finally {
+ statement.close()
+ }
+ }
+
+ /**
+ * Process index properties and return tuple of indexType and list of the other index properties.
+ */
+ def processIndexProperties(
+ properties: util.Map[String, String],
+ catalogName: String
+ ): (String, Array[String]) = {
+ var indexType = ""
+ val indexPropertyList: ArrayBuffer[String] = ArrayBuffer[String]()
+ val supportedIndexTypeList = getSupportedIndexTypeList(catalogName)
+
+ if (!properties.isEmpty) {
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (containsIndexTypeIgnoreCase(supportedIndexTypeList, v)) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ s" The supported Index Types are: ${supportedIndexTypeList.mkString(" AND ")}")
+ }
+ } else {
+ indexPropertyList.append(convertPropertyPairToString(catalogName, k, v))
+ }
+ }
+ }
+ (indexType, indexPropertyList.toArray)
+ }
+
+ def containsIndexTypeIgnoreCase(supportedIndexTypeList: Array[String], value: String): Boolean = {
+ if (supportedIndexTypeList.isEmpty) {
+ throw new UnsupportedOperationException(s"None of index type is supported.")
+ }
+ for (indexType <- supportedIndexTypeList) {
+ if (value.equalsIgnoreCase(indexType)) return true
+ }
+ false
+ }
+
+ def getSupportedIndexTypeList(catalogName: String): Array[String] = {
+ catalogName match {
+ case "mysql" => Array("BTREE", "HASH")
+ case "postgresql" => Array("BTREE", "HASH", "BRIN")
+ case _ => Array.empty
+ }
+ }
Review comment:
I think it may be better to put this in `JdbcDialect` and let the subclasses implement this. WDYT? @cloud-fan
--
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] dchvn commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r762668281
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -71,6 +71,14 @@ private[v2] trait V2JDBCTest extends SharedSparkSession with DockerIntegrationFu
def testCreateTableWithProperty(tbl: String): Unit = {}
+ def indexOptions(catalogName: String): String = {
+ catalogName match {
+ case "mysql" => "KEY_BLOCK_SIZE=10"
+ case "postgresql" => "FILLFACTOR=70"
+ case _ => ""
+ }
+ }
Review comment:
Thank you for clarifying.
--
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 #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-987620470
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50449/
--
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] SparkQA commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-987733097
**[Test build #145974 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145974/testReport)** for PR 34673 at commit [`67338d4`](https://github.com/apache/spark/commit/67338d476bd340f436e7ad7cb4020375a7230f0e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] kevin-Murphy-hi commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
kevin-Murphy-hi commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-987639705
退订
dch_nguyen ***@***.***> 于2021年11月20日周六 13:16写道：
> What changes were proposed in this pull request?
>
> Implementing createIndex/IndexExists/dropIndex in DS V2 JDBC for Postgres
> dialect.
> Why are the changes needed?
>
> This is a subtask of the V2 Index support. This PR implements createIndex,
> IndexExists and dropIndex. After review for some changes in this PR, I
> will create new PR for listIndexs, or add it in this PR.
>
> This PR only implements createIndex, IndexExists and dropIndex in
> Postgres dialect.
> Does this PR introduce *any* user-facing change?
>
> Yes, createIndex/IndexExists/dropIndex in DS V2 JDBC
> How was this patch tested?
>
> New test.
> ------------------------------
> You can view, comment on, or merge this pull request online at:
>
> https://github.com/apache/spark/pull/34673
> Commit Summary
>
> - baae935
> <https://github.com/apache/spark/pull/34673/commits/baae935fc0c0cc47c4397be662ac7d36dfbbe8b0>
> Dsv2_index_postgres
>
> File Changes
>
> (5 files <https://github.com/apache/spark/pull/34673/files>)
>
> - *M*
> external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/PostgresIntegrationSuite.scala
> <https://github.com/apache/spark/pull/34673/files#diff-236e33febf7e76c639a064156dc3f52abf612660015d7c4b99c10acc78371752>
> (2)
> - *M*
> external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
> <https://github.com/apache/spark/pull/34673/files#diff-7fe271b3eb77d236f4d92ea6ec2b6715e539a7e1c5cc71df0d056900fdd159df>
> (4)
> - *M*
> sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
> <https://github.com/apache/spark/pull/34673/files#diff-c3859e97335ead4b131263565c987d877bea0af3adbd6c5bf2d3716768d2e083>
> (25)
> - *M*
> sql/core/src/main/scala/org/apache/spark/sql/jdbc/MySQLDialect.scala
> <https://github.com/apache/spark/pull/34673/files#diff-02d2c1918aad70fcc80bf57cc8b8adfa58c144f4b81d6281e3557ed023ccefb1>
> (9)
> - *M*
> sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala
> <https://github.com/apache/spark/pull/34673/files#diff-374d2da08d88160979e0fedb2ded14e5fa8ed04942a94cdd9672775332642226>
> (86)
>
> Patch Links:
>
> - https://github.com/apache/spark/pull/34673.patch
> - https://github.com/apache/spark/pull/34673.diff
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/spark/pull/34673>, or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AOCKMJLI6G2KH5DL5AY5OITUM4VLHANCNFSM5INRUZHA>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
>
--
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] cloud-fan commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r767468635
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -1073,6 +1076,81 @@ object JdbcUtils extends Logging with SQLConfHelper {
}
}
+ /**
+ * Check if index exists in a table
+ */
+ def checkIfIndexExists(
+ conn: Connection,
+ sql: String,
+ options: JDBCOptions): Boolean = {
+ val statement = conn.createStatement
+ try {
+ statement.setQueryTimeout(options.queryTimeout)
+ val rs = statement.executeQuery(sql)
+ rs.next
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
+ } finally {
+ statement.close()
+ }
+ }
+
+ /**
+ * Process index properties and return tuple of indexType and list of the other index properties.
+ */
+ def processIndexProperties(
+ properties: util.Map[String, String],
+ catalogName: String
+ ): (String, Array[String]) = {
+ var indexType = ""
+ val indexPropertyList: ArrayBuffer[String] = ArrayBuffer[String]()
+ val supportedIndexTypeList = getSupportedIndexTypeList(catalogName)
+
+ if (!properties.isEmpty) {
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (containsIndexTypeIgnoreCase(supportedIndexTypeList, v)) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ s" The supported Index Types are: ${supportedIndexTypeList.mkString(" AND ")}")
+ }
+ } else {
+ indexPropertyList.append(convertPropertyPairToString(catalogName, k, v))
+ }
+ }
+ }
+ (indexType, indexPropertyList.toArray)
+ }
+
+ def containsIndexTypeIgnoreCase(supportedIndexTypeList: Array[String], value: String): Boolean = {
+ if (supportedIndexTypeList.isEmpty) {
+ throw new UnsupportedOperationException(s"None of index type is supported.")
+ }
+ for (indexType <- supportedIndexTypeList) {
+ if (value.equalsIgnoreCase(indexType)) return true
+ }
+ false
+ }
+
+ def getSupportedIndexTypeList(catalogName: String): Array[String] = {
+ catalogName match {
+ case "mysql" => Array("BTREE", "HASH")
+ case "postgresql" => Array("BTREE", "HASH", "BRIN")
+ case _ => Array.empty
+ }
+ }
Review comment:
Yea +1 to put it in `JdbcDialect`, instead of hardcoding the dialect names here.
--
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] dchvn commented on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
dchvn commented on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-992078483
cc @huaxingao @cloud-fan. Please took a look if you have time. Many thanks!
--
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] cloud-fan commented on a change in pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34673:
URL: https://github.com/apache/spark/pull/34673#discussion_r767472919
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -1073,6 +1076,81 @@ object JdbcUtils extends Logging with SQLConfHelper {
}
}
+ /**
+ * Check if index exists in a table
+ */
+ def checkIfIndexExists(
+ conn: Connection,
+ sql: String,
+ options: JDBCOptions): Boolean = {
+ val statement = conn.createStatement
+ try {
+ statement.setQueryTimeout(options.queryTimeout)
+ val rs = statement.executeQuery(sql)
+ rs.next
+ } catch {
+ case _: Exception =>
+ logWarning("Cannot retrieved index info.")
+ false
+ } finally {
+ statement.close()
+ }
+ }
+
+ /**
+ * Process index properties and return tuple of indexType and list of the other index properties.
+ */
+ def processIndexProperties(
+ properties: util.Map[String, String],
+ catalogName: String
+ ): (String, Array[String]) = {
+ var indexType = ""
+ val indexPropertyList: ArrayBuffer[String] = ArrayBuffer[String]()
+ val supportedIndexTypeList = getSupportedIndexTypeList(catalogName)
+
+ if (!properties.isEmpty) {
+ properties.asScala.foreach { case (k, v) =>
+ if (k.equals(SupportsIndex.PROP_TYPE)) {
+ if (containsIndexTypeIgnoreCase(supportedIndexTypeList, v)) {
+ indexType = s"USING $v"
+ } else {
+ throw new UnsupportedOperationException(s"Index Type $v is not supported." +
+ s" The supported Index Types are: ${supportedIndexTypeList.mkString(" AND ")}")
+ }
+ } else {
+ indexPropertyList.append(convertPropertyPairToString(catalogName, k, v))
+ }
+ }
+ }
+ (indexType, indexPropertyList.toArray)
+ }
+
+ def containsIndexTypeIgnoreCase(supportedIndexTypeList: Array[String], value: String): Boolean = {
+ if (supportedIndexTypeList.isEmpty) {
+ throw new UnsupportedOperationException(s"None of index type is supported.")
+ }
+ for (indexType <- supportedIndexTypeList) {
+ if (value.equalsIgnoreCase(indexType)) return true
+ }
+ false
+ }
+
+ def getSupportedIndexTypeList(catalogName: String): Array[String] = {
+ catalogName match {
+ case "mysql" => Array("BTREE", "HASH")
+ case "postgresql" => Array("BTREE", "HASH", "BRIN")
+ case _ => Array.empty
+ }
+ }
Review comment:
So the only benefit of adding this API in `JdbcDialect` is to share the code of checking unsupported index type earlier, which I don't think is worthwhile.
--
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] SparkQA removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-995547503
**[Test build #146271 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146271/testReport)** for PR 34673 at commit [`36ee897`](https://github.com/apache/spark/commit/36ee897e7a12e4631f75d6d8cb057bd922c71437).
--
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 removed a comment on pull request #34673: [SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-995813458
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146271/
--
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 removed a comment on pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974610792
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49947/
--
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 removed a comment on pull request #34673: [WIP][SPARK-37343][SQL] Implement createIndex, IndexExists and dropIndex in JDBC (Postgres dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34673:
URL: https://github.com/apache/spark/pull/34673#issuecomment-974630135
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145475/
--
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