This is an automated email from the ASF dual-hosted git repository. maxgekk pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push: new e00c1209021 [SPARK-40136][SQL] Fix the fragment of SQL query contexts e00c1209021 is described below commit e00c12090217f1fa378cd99c1cc596033a065326 Author: Max Gekk <max.g...@gmail.com> AuthorDate: Thu Aug 18 21:31:07 2022 +0300 [SPARK-40136][SQL] Fix the fragment of SQL query contexts ### What changes were proposed in this pull request? In the PR, I propose to change implementation of `fragment` in `SQLQueryContext` to get the fragment correctly. Before the changes, `fragment` misses the last char because `originStopIndex.get` points to the last char of fragments but `substring()` gets a sub-string up to `endIndex - 1`. ### Why are the changes needed? It fixes a bug. ### Does this PR introduce _any_ user-facing change? Yes. ### How was this patch tested? By running new test suite: ``` $ build/sbt "test:testOnly *SQLQueryContextSuite" $ build/sbt "test:testOnly *TreeNodeSuite" $ build/sbt "test:testOnly *QueryExecutionAnsiErrorsSuite" ``` Closes #37566 from MaxGekk/fix-fragment. Authored-by: Max Gekk <max.g...@gmail.com> Signed-off-by: Max Gekk <max.g...@gmail.com> --- .../spark/sql/catalyst/trees/SQLQueryContext.scala | 2 +- .../sql/catalyst/trees/SQLQueryContextSuite.scala | 49 ++++++++++++++++++++++ .../spark/sql/catalyst/trees/TreeNodeSuite.scala | 2 +- .../sql/errors/QueryExecutionAnsiErrorsSuite.scala | 12 +++--- 4 files changed, 57 insertions(+), 8 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/SQLQueryContext.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/SQLQueryContext.scala index a8806dbad4d..2f6b0f885a0 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/SQLQueryContext.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/SQLQueryContext.scala @@ -120,7 +120,7 @@ case class SQLQueryContext( if (!isValid) { "" } else { - sqlText.get.substring(originStartIndex.get, originStopIndex.get) + sqlText.get.substring(originStartIndex.get, originStopIndex.get + 1) } } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/trees/SQLQueryContextSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/trees/SQLQueryContextSuite.scala new file mode 100644 index 00000000000..61fe90093d2 --- /dev/null +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/trees/SQLQueryContextSuite.scala @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.catalyst.trees + +import org.apache.spark.SparkFunSuite +import org.apache.spark.sql.catalyst.plans.SQLHelper + +class SQLQueryContextSuite extends SparkFunSuite with SQLHelper { + + test("SPARK-40136: the length of the fragment in query context") { + def getFragment(sqlText: String, start: Int, stop: Int): String = { + val context = SQLQueryContext( + line = None, + startPosition = None, + originStartIndex = Some(start), + originStopIndex = Some(stop), + sqlText = Some(sqlText), + originObjectType = None, + originObjectName = None) + context.fragment + } + Seq( + ("select 1 / 0", 7, 11) -> "1 / 0", + ("select 1 / 0", 7, 7) -> "1", + ("", 7, 7) -> "", + // Empty fragment for invalid indexes + ("select 1 / 0", -1, 11) -> "", + ("select 1 / 0", 7, 12) -> "", + ("select 1 / 0", 11, 7) -> "" + ).foreach { case ((sqlText, start, stop), expectedFragment) => + assert(getFragment(sqlText, start, stop) === expectedFragment) + } + } +} diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/trees/TreeNodeSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/trees/TreeNodeSuite.scala index 442bd01aa5b..bef88a7c0a3 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/trees/TreeNodeSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/trees/TreeNodeSuite.scala @@ -888,7 +888,7 @@ class TreeNodeSuite extends SparkFunSuite with SQLHelper { val expectedFragment = """cast('a' |as /* comment */ - |int)""".stripMargin + |int),""".stripMargin assert(origin.context.summary == expectedSummary) assert(origin.context.startIndex == origin.startIndex.get) assert(origin.context.stopIndex == origin.stopIndex.get) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/errors/QueryExecutionAnsiErrorsSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/errors/QueryExecutionAnsiErrorsSuite.scala index 1ebca5f5f6a..837880a1d6d 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/errors/QueryExecutionAnsiErrorsSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/errors/QueryExecutionAnsiErrorsSuite.scala @@ -49,7 +49,7 @@ class QueryExecutionAnsiErrorsSuite extends QueryTest with QueryErrorsSuiteBase errorClass = "DIVIDE_BY_ZERO", sqlState = "22012", parameters = Map("config" -> ansiConf), - context = ExpectedContext(fragment = "6/", start = 7, stop = 9)) + context = ExpectedContext(fragment = "6/0", start = 7, stop = 9)) } test("INTERVAL_DIVIDED_BY_ZERO: interval divided by zero") { @@ -60,7 +60,7 @@ class QueryExecutionAnsiErrorsSuite extends QueryTest with QueryErrorsSuiteBase errorClass = "INTERVAL_DIVIDED_BY_ZERO", sqlState = "22012", parameters = Map.empty[String, String], - context = ExpectedContext(fragment = "interval 1 day / ", start = 7, stop = 24)) + context = ExpectedContext(fragment = "interval 1 day / 0", start = 7, stop = 24)) } test("INVALID_FRACTION_OF_SECOND: in the function make_timestamp") { @@ -86,7 +86,7 @@ class QueryExecutionAnsiErrorsSuite extends QueryTest with QueryErrorsSuiteBase "scale" -> "1", "config" -> ansiConf), context = ExpectedContext( - fragment = "CAST('66666666666666.666' AS DECIMAL(8, 1)", + fragment = "CAST('66666666666666.666' AS DECIMAL(8, 1))", start = 7, stop = 49)) } @@ -98,7 +98,7 @@ class QueryExecutionAnsiErrorsSuite extends QueryTest with QueryErrorsSuiteBase }, errorClass = "INVALID_ARRAY_INDEX", parameters = Map("indexValue" -> "8", "arraySize" -> "5", "ansiConfig" -> ansiConf), - context = ExpectedContext(fragment = "array(1, 2, 3, 4, 5)[8", start = 7, stop = 29)) + context = ExpectedContext(fragment = "array(1, 2, 3, 4, 5)[8]", start = 7, stop = 29)) } test("INVALID_ARRAY_INDEX_IN_ELEMENT_AT: element_at from array") { @@ -109,7 +109,7 @@ class QueryExecutionAnsiErrorsSuite extends QueryTest with QueryErrorsSuiteBase errorClass = "INVALID_ARRAY_INDEX_IN_ELEMENT_AT", parameters = Map("indexValue" -> "8", "arraySize" -> "5", "ansiConfig" -> ansiConf), context = ExpectedContext( - fragment = "element_at(array(1, 2, 3, 4, 5), 8", + fragment = "element_at(array(1, 2, 3, 4, 5), 8)", start = 7, stop = 41)) } @@ -126,7 +126,7 @@ class QueryExecutionAnsiErrorsSuite extends QueryTest with QueryErrorsSuiteBase "targetType" -> "\"DOUBLE\"", "ansiConfig" -> ansiConf), context = ExpectedContext( - fragment = "CAST('111111111111xe23' AS DOUBLE", + fragment = "CAST('111111111111xe23' AS DOUBLE)", start = 7, stop = 40)) } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org