srielau commented on code in PR #59868:
URL: https://github.com/apache/spark/pull/59868#discussion_r4420526359
##########
sql/core/src/main/scala/org/apache/spark/sql/catalyst/parser/ParseSqlResult.scala:
##########
@@ -123,9 +134,85 @@ object ParseSqlResult {
JObject(fields.toList)
}
+ /**
+ * 1-based Unicode code-point starts for `statements` in `batch`. Statements
+ * are in ascending UTF-16 start order, so one left-to-right pass is enough:
+ * each count covers only the gap since the previous start.
+ */
+ private def charStarts(
+ batch: String,
+ statements: Seq[PositionedSqlStatement]): Seq[Int] = {
+ var utf16 = 0
+ var codePoints = 0
+ statements.map { segment =>
+ val startUtf16 = math.max(0, math.min(segment.start, batch.length))
+ if (startUtf16 >= utf16) {
+ codePoints += batch.codePointCount(utf16, startUtf16)
+ } else {
+ codePoints = batch.codePointCount(0, startUtf16)
+ }
+ utf16 = startUtf16
+ codePoints + 1
+ }
+ }
+
+ /** Unicode code-point length of the trimmed statement text. */
+ private def charLength(segment: PositionedSqlStatement): Int = {
+ val text = segment.statement
+ text.codePointCount(0, text.length)
+ }
+
+ /**
+ * Keep statement-level `queryContext` indexes inside the statement.
+ * In-range contexts keep the shared renderer's fragment. A context that
+ * falls outside the statement is widened to cover all of it. Contexts of
+ * another object (a view body) are left as is.
+ */
+ private def rewriteErrorCharIndexes(errorObj: JObject, statement: String):
JObject = {
+ JObject(errorObj.obj.map {
+ case ("queryContext", JArray(contexts)) =>
+ "queryContext" -> JArray(contexts.map {
+ case ctx: JObject => rewriteQueryContextIndexes(ctx, statement)
+ case other => other
+ }.toList)
+ case other => other
+ })
+ }
+
+ private def rewriteQueryContextIndexes(ctx: JObject, statement: String):
JObject = {
+ val objectType = ctx.obj.collectFirst { case ("objectType", JString(t)) =>
t }
+ val start = ctx.obj.collectFirst { case ("startIndex", JInt(n)) => n.toInt
}
+ val stop = ctx.obj.collectFirst { case ("stopIndex", JInt(n)) => n.toInt }
+ (objectType, start, stop) match {
+ case (Some(""), Some(from), Some(to)) =>
+ val length = statement.codePointCount(0, statement.length)
+ if (from >= 1 && to >= from && to <= length) {
+ ctx
+ } else {
+ JObject(ctx.obj.map {
+ case ("startIndex", _) => "startIndex" -> JInt(1)
+ case ("stopIndex", _) => "stopIndex" -> JInt(length)
+ case ("fragment", _) =>
+ "fragment" -> JString(codePointSlice(statement, 0, length))
Review Comment:
Done in 880e56fa548. The widening branch now emits `JString(statement)` and
`codePointSlice` is gone. In-range error JSON is unchanged.
##########
sql/core/src/main/scala/org/apache/spark/sql/scripting/SqlScriptingExecutionNode.scala:
##########
@@ -185,8 +185,8 @@ class SingleStatementExec(
* SQL query text.
*/
def getText: String = {
- assert(origin.sqlText.isDefined && origin.startIndex.isDefined &&
origin.stopIndex.isDefined)
- origin.sqlText.get.substring(origin.startIndex.get, origin.stopIndex.get +
1)
+ assert(origin.sqlTextSlice.isDefined)
Review Comment:
Added call-site tests in 880e56fa548:
- `INVALID_BOOLEAN_STATEMENT` after `SELECT '<emoji>'` names exactly `1`.
- `NULL_QUERY_STRING_EXECUTE_IMMEDIATE` with an emoji in a comment before
the variable names exactly `` `q` ``.
- `SparkSqlParser` whole-text `stopIndex` is observed through
`parsePlanWithParameters`: a non-empty parameter context still installs that
origin, and a syntax error after two emojis keeps a context covering the full
original SQL. A parameterized `FRUM` query never reaches this origin because
substitution parses first and throws.
Existing ASCII cases for the boolean-condition and typed-literal errors are
unchanged.
##########
sql/core/src/main/scala/org/apache/spark/sql/catalyst/parser/ParseSqlResult.scala:
##########
@@ -123,9 +134,83 @@ object ParseSqlResult {
JObject(fields.toList)
}
+ /**
+ * 1-based Unicode code-point starts for `statements` in `batch`. Statements
+ * are in ascending UTF-16 start order, so one left-to-right pass is enough:
+ * each count covers only the gap since the previous start.
+ */
+ private def charStarts(
+ batch: String,
+ statements: Seq[PositionedSqlStatement]): Seq[Int] = {
+ var utf16 = 0
+ var codePoints = 0
+ statements.map { segment =>
+ val startUtf16 = math.max(0, math.min(segment.start, batch.length))
+ if (startUtf16 >= utf16) {
+ codePoints += batch.codePointCount(utf16, startUtf16)
+ } else {
+ codePoints = batch.codePointCount(0, startUtf16)
+ }
+ utf16 = startUtf16
+ codePoints + 1
+ }
+ }
+
+ /** Unicode code-point length of the trimmed statement text. */
+ private def charLength(segment: PositionedSqlStatement): Int = {
+ val text = segment.statement
+ text.codePointCount(0, text.length)
+ }
+
+ /**
+ * Recompute the fragment of each statement `queryContext` by code points.
+ * The indexes are parser (code-point) offsets, but [[SQLQueryContext]]
+ * slices its fragment by UTF-16 units, which is off after a supplementary
+ * character. A context that falls outside the statement covers all of it.
+ * Contexts of another object (a view body) are left as is.
+ */
+ private def rewriteErrorCharIndexes(errorObj: JObject, statement: String):
JObject = {
+ JObject(errorObj.obj.map {
+ case ("queryContext", JArray(contexts)) =>
+ "queryContext" -> JArray(contexts.map {
+ case ctx: JObject => rewriteQueryContextIndexes(ctx, statement)
+ case other => other
+ }.toList)
+ case other => other
+ })
+ }
+
+ private def rewriteQueryContextIndexes(ctx: JObject, statement: String):
JObject = {
+ val objectType = ctx.obj.collectFirst { case ("objectType", JString(t)) =>
t }
+ val start = ctx.obj.collectFirst { case ("startIndex", JInt(n)) => n.toInt
}
+ val stop = ctx.obj.collectFirst { case ("stopIndex", JInt(n)) => n.toInt }
+ (objectType, start, stop) match {
+ case (Some(""), Some(from), Some(to)) =>
+ val length = statement.codePointCount(0, statement.length)
+ val (first, last) =
+ if (from >= 1 && to >= from && to <= length) (from, to) else (1,
length)
+ JObject(ctx.obj.map {
+ case ("startIndex", _) => "startIndex" -> JInt(first)
+ case ("stopIndex", _) => "stopIndex" -> JInt(last)
+ case ("fragment", _) => "fragment" ->
JString(codePointSlice(statement, first - 1, last))
Review Comment:
`SparkSqlParser.stopIndex` is now pinned through `parsePlanWithParameters`
in 880e56fa548, not only by constructing an `Origin` by hand. The check fails
if that origin goes back to a UTF-16 length.
On recounting start/length: I would still keep the current implementation.
Reusing one `codePointCount` per statement is a small cleanup, but it is not
needed for correctness and I would rather not mix it into this bugfix.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
