Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,6 @@ class PostgresIntegrationSuite extends DockerJDBCIntegrationSuite with V2JDBCTes
}

override def supportsTableSample: Boolean = true

override def supportsIndex: Boolean = true
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depend on type of jdbc dialect, we change the index options for test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a method for this and implement it in each concrete test suite? e.g. def indexOptions: String = ""

withTable(s"$catalogName.new_table") {
sql(s"CREATE TABLE $catalogName.new_table(col1 INT, col2 INT, col3 INT," +
" col4 INT, col5 INT)")
Expand All @@ -208,11 +210,11 @@ private[v2] trait V2JDBCTest extends SharedSparkSession with DockerIntegrationFu
sql(s"CREATE index i1 ON $catalogName.new_table USING $indexType (col1)")
}.getMessage
assert(m.contains(s"Index Type $indexType is not supported." +
s" The supported Index Types are: BTREE and HASH"))
s" The supported Index Types are:"))

sql(s"CREATE index i1 ON $catalogName.new_table USING BTREE (col1)")
sql(s"CREATE index i2 ON $catalogName.new_table (col2, col3, col5)" +
s" OPTIONS (KEY_BLOCK_SIZE=10)")
s" OPTIONS ($indexOptions)")

assert(jdbcTable.indexExists("i1") == true)
assert(jdbcTable.indexExists("i2") == true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import java.util
import java.util.Locale
import java.util.concurrent.TimeUnit

import scala.collection.JavaConverters._
import scala.util.Try
import scala.util.control.NonFatal

Expand All @@ -38,6 +39,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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: combine this with the next line

import org.apache.spark.sql.connector.catalog.index.TableIndex
import org.apache.spark.sql.connector.expressions.NamedReference
import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryExecutionErrors}
Expand Down Expand Up @@ -1025,7 +1027,7 @@ object JdbcUtils extends Logging with SQLConfHelper {
options: JDBCOptions): Unit = {
val dialect = JdbcDialects.get(options.url)
executeStatement(conn, options,
dialect.createIndex(indexName, tableName, columns, columnsProperties, properties))
dialect.createIndex(indexName, tableName, columns, columnsProperties, properties, options))
}

/**
Expand Down Expand Up @@ -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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be ArrayBuffer

val supportedIndexTypeList = dialect.getSupportedIndexTypeList()

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 = indexPropertyList :+ dialect.convertPropertyPairToString(k, v)
}
}
}
(indexType, indexPropertyList)
}

def containsIndexTypeIgnoreCase(supportedIndexTypeList: Array[String], value: String): Boolean = {
if (supportedIndexTypeList.isEmpty) {
throw new UnsupportedOperationException(s"None of index type is supported.")

@cloud-fan cloud-fan Dec 13, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be more user-facing: Cannot specify 'USING index_type' in 'CREATE INDEX'

}
for (indexType <- supportedIndexTypeList) {
if (value.equalsIgnoreCase(indexType)) return true
}
false
}

def executeQuery(conn: Connection, options: JDBCOptions, sql: String): ResultSet = {
val statement = conn.createStatement
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,8 @@ abstract class JdbcDialect extends Serializable with Logging{
tableName: String,
columns: Array[NamedReference],
columnsProperties: util.Map[NamedReference, util.Map[String, String]],
properties: util.Map[String, String]): String = {
properties: util.Map[String, String],
options: JDBCOptions): String = {
throw new UnsupportedOperationException("createIndex is not supported")
}

Expand Down Expand Up @@ -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 = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

s"$key $value"
}

/**
* Return list of supported index type
*/
def getSupportedIndexTypeList(): Array[String] = Array.empty

/**
* returns the LIMIT clause for the SELECT statement
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,10 @@ import java.sql.{Connection, SQLException, Types}
import java.util
import java.util.Locale

import scala.collection.JavaConverters._

import org.apache.spark.sql.AnalysisException
import org.apache.spark.sql.catalyst.SQLConfHelper
import org.apache.spark.sql.catalyst.analysis.{IndexAlreadyExistsException, NoSuchIndexException}
import org.apache.spark.sql.connector.catalog.index.{SupportsIndex, TableIndex}
import org.apache.spark.sql.connector.catalog.index.TableIndex
import org.apache.spark.sql.connector.expressions.{FieldReference, NamedReference}
import org.apache.spark.sql.errors.QueryExecutionErrors
import org.apache.spark.sql.execution.datasources.jdbc.{JDBCOptions, JdbcUtils}
Expand Down Expand Up @@ -118,27 +116,15 @@ private case object MySQLDialect extends JdbcDialect with SQLConfHelper {
tableName: String,
columns: Array[NamedReference],
columnsProperties: util.Map[NamedReference, util.Map[String, String]],
properties: util.Map[String, String]): String = {
properties: util.Map[String, String],
options: JDBCOptions): String = {
val columnList = columns.map(col => quoteIdentifier(col.fieldNames.head))
var indexProperties: String = ""
var indexType = ""
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")
}
} else {
indexProperties = indexProperties + " " + s"$k $v"
}
}
}
val (indexType, indexPropertyList) = JdbcUtils.processIndexProperties(properties, options)

// columnsProperties doesn't apply to MySQL so it is ignored
s"CREATE INDEX ${quoteIdentifier(indexName)} $indexType ON" +
s" ${quoteIdentifier(tableName)} (${columnList.mkString(", ")}) $indexProperties"
s" ${quoteIdentifier(tableName)} (${columnList.mkString(", ")})" +
s" ${indexPropertyList.mkString(" ")}"
}

// SHOW INDEX syntax
Expand All @@ -148,21 +134,8 @@ private case object MySQLDialect extends JdbcDialect with SQLConfHelper {
indexName: String,
tableName: String,
options: JDBCOptions): Boolean = {
val sql = s"SHOW INDEXES FROM ${quoteIdentifier(tableName)}"
try {
val rs = JdbcUtils.executeQuery(conn, options, sql)
while (rs.next()) {
val retrievedIndexName = rs.getString("key_name")
if (conf.resolver(retrievedIndexName, indexName)) {
return true
}
}
false
} catch {
case _: Exception =>
logWarning("Cannot retrieved index info.")
false
}
val sql = s"SHOW INDEXES FROM ${quoteIdentifier(tableName)} WHERE key_name = '$indexName'"
JdbcUtils.checkIfIndexExists(conn, sql, options)
}

override def dropIndex(indexName: String, tableName: String): String = {
Expand Down Expand Up @@ -222,4 +195,8 @@ private case object MySQLDialect extends JdbcDialect with SQLConfHelper {
case _ => super.classifyException(message, e)
}
}

override def getSupportedIndexTypeList(): Array[String] = {
Array("BTREE", "HASH")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,20 @@

package org.apache.spark.sql.jdbc

import java.sql.{Connection, Types}
import java.sql.{Connection, SQLException, Types}
import java.util
import java.util.Locale

import org.apache.spark.sql.AnalysisException
import org.apache.spark.sql.catalyst.SQLConfHelper
import org.apache.spark.sql.catalyst.analysis.{IndexAlreadyExistsException, NoSuchIndexException}
import org.apache.spark.sql.connector.expressions.NamedReference
import org.apache.spark.sql.execution.datasources.jdbc.{JDBCOptions, JdbcUtils}
import org.apache.spark.sql.execution.datasources.v2.TableSampleInfo
import org.apache.spark.sql.types._


private object PostgresDialect extends JdbcDialect {
private object PostgresDialect extends JdbcDialect with SQLConfHelper {

override def canHandle(url: String): Boolean =
url.toLowerCase(Locale.ROOT).startsWith("jdbc:postgresql")
Expand Down Expand Up @@ -164,4 +169,65 @@ 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],
options: JDBCOptions): String = {
val columnList = columns.map(col => quoteIdentifier(col.fieldNames.head))
var indexProperties = ""
val (indexType, indexPropertyList) = JdbcUtils.processIndexProperties(properties, options)

if (indexPropertyList.nonEmpty) {
indexProperties = "WITH (" + indexPropertyList.mkString(", ") + ")"
}

s"CREATE INDEX ${quoteIdentifier(indexName)} ON ${quoteIdentifier(tableName)}" +
s" $indexType (${columnList.mkString(", ")}) $indexProperties"
}

// 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' AND" +
s" indexname = '$indexName'"
JdbcUtils.checkIfIndexExists(conn, sql, options)
}

// 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))
Comment on lines +216 to +217

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use Postgres error codes to handle the exception, but getErrorCode returns 0. So I use getSQLState instead.

case _ => super.classifyException(message, e)
}
case unsupported: UnsupportedOperationException => throw unsupported
case _ => super.classifyException(message, e)
}
}

override def convertPropertyPairToString(key: String, value: String): String = {
s"$key = $value"
}

override def getSupportedIndexTypeList(): Array[String] = {
Array("BTREE", "HASH", "BRIN")
}
}