cloud-fan commented on code in PR #59868:
URL: https://github.com/apache/spark/pull/59868#discussion_r4407271872
##########
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:
**Non-blocking (P2):** This rewrite corrects the fragment only in
parse_sql's copy of the error. The unit mismatch it works around is in
`SQLQueryContext`, which this PR leaves unchanged: parser origins carry ANTLR
code-point indexes, but `fragment`, `summary`, and `isValid` treat them as
UTF-16 offsets. The regular SQL error path therefore still reports the wrong
text for any parser-origin context after a supplementary character. For `SELECT
'<emoji>' AS e, DATE 'bad'`, parse_sql now reports the fragment `DATE 'bad'`,
while `spark.sql` still reports ` DATE 'bad` in the STANDARD error JSON and
`QueryContext.fragment()`, and its PRETTY caret sits one column to the left. As
a result, parse_sql's nested error object, which the function docs describe as
STANDARD format, now disagrees with Spark's STANDARD error for the same
exception. Only error text is affected; query results are not.
**Recommended change:** Make SQLQueryContext treat startIndex and stopIndex
as Unicode code-point offsets into its SQL text, computing the fragment, the
summary caret, and the validity check by code points, with a direct path when
the text has no supplementary characters. Switch the whole-text origins for
parameter substitution, EXECUTE IMMEDIATE, and SQL scripting case conditions
from UTF-16 lengths to code-point counts, and make the parameter position
mapper measure each substituted value in code points instead of UTF-16 units,
so every index that reaches the renderer uses the parser's unit. Then remove
parse_sql's per-context fragment re-slicing, which the shared renderer now
produces, keeping its out-of-range widening guard and its start/length
conversion. Add query-context and error-message tests for errors after a
supplementary character on the regular SQL path, including a parameterized
query whose parameter value contains one.
**Why this works:** Parser indexes are already code points, so interpreting
them as code points at the single renderer makes the fragment, caret, and
validity agree with the indexes for every consumer at once. Converting the
remaining UTF-16 whole-text producers to code points, and measuring substituted
parameter values in code points when mapping positions back to the original
text, gives every index one unit, so no consumer needs its own correction and
parse_sql's nested error object matches Spark's STANDARD error for the same
exception.
**Scope:** Fix the unit at the shared query-context renderer, its producers,
and the parameter position mapping, then simplify parse_sql's error rewrite to
rely on it.
**Compatibility:** Parser-produced startIndex and stopIndex values,
parse_sql's start/length output, and every error surface's output for text
without supplementary characters.
**Risks:** Whole-text contexts from parameter substitution, EXECUTE
IMMEDIATE, and scripting case conditions report a smaller stopIndex for text
containing supplementary characters, and mapped positions after a parameter
value containing one move to their code-point offsets, so an external consumer
that sliced by the old UTF-16 indexes would see different bounds; text and
parameter values without supplementary characters are unchanged, and the
producer and mapper edits can be reverted only together with the renderer
change. A context whose indexes exceed its text's code-point length, reachable
only when variable substitution lengthens the parsed text, is dropped by the
code-point validity check instead of reaching parse_sql's whole-statement
widening. Rendering a fragment or caret for text with characters above U+00FF
adds a code-point offset walk, which runs only when an error context is
rendered.
**Constraints:** Parser-produced startIndex and stopIndex values must not
change, because clients already receive them and parse_sql documents them as
code points. After the change every Origin producer that sets startIndex or
stopIndex, and the parameter position mapper that translates them, must use
code points; any producer left on UTF-16 lengths would again truncate or
misplace fragments for text with supplementary characters, and a mapper left on
UTF-16 value lengths would truncate the whole-text context of a parameterized
query whose parameter values contain them.
**Exclusions:**
- DataFrame-API query contexts, which carry call sites rather than SQL
indexes.
- How parse_sql computes statement start and length, including whether the
statement splitter should carry code-point offsets, which is raised separately
as an author choice.
**Success:** For a parser-origin error after a supplementary character, the
QueryContext API fragment, the STANDARD error JSON fragment, and parse_sql's
nested error fragment are identical and equal the text over the context's
code-point index range. The PRETTY summary caret underlines exactly the
context's code-point range. Whole-text contexts for parameter substitution,
EXECUTE IMMEDIATE, scripting case conditions, and the parse_sql fallback cover
their full text with a code-point stopIndex, including a parameterized query
whose parameter values contain supplementary characters. In a parameterized
query, an error location after a parameter value that contains a supplementary
character maps to the code-point offset of the same location in the original
text. Errors in text without supplementary characters produce the same indexes,
fragments, and summaries on every surface as before.
**Verification:**
- **Regression:** On the regular SQL error path, a statement with a
supplementary character before an error location, such as a typed-literal error
after an emoji string literal, reports a fragment equal to the code-point range
of its indexes through both the STANDARD error JSON and the QueryContext API,
with an aligned summary caret.
- **Behavior:** parse_sql still reports the same code-point indexes and
fragments for syntax, typed-literal, and scripting errors after a supplementary
character once its local fragment re-slicing is removed.
- **Behavior:** Whole-text contexts for a parameterized query, EXECUTE
IMMEDIATE, and a scripting case condition whose text contains a supplementary
character report the full text as fragment and a code-point stopIndex, and a
parameterized query whose parameter value contains a supplementary character
reports both its whole-text context and a parser error after that value at the
original text's code-point offsets.
- **Compatibility:** Errors in text without supplementary characters produce
identical startIndex, stopIndex, fragment, and summary on every surface before
and after the change.
--
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]
