HyukjinKwon commented on code in PR #59097:
URL: https://github.com/apache/spark/pull/59097#discussion_r4172988494
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/variant/VariantExpressionSuite.scala:
##########
@@ -1640,6 +1666,22 @@ class VariantExpressionSuite extends SparkFunSuite with
ExpressionEvalHelper {
checkInsertRecoverableError("""{"a.b": 5}""", """$['a.b'].c""", Literal(2),
"VARIANT_PATH_TYPE_MISMATCH",
Map("path" -> "$['a.b'].c", "failedAt" -> "$['a.b']", "functionName" ->
"`variant_insert`"))
+ checkInsertRecoverableError("""{"a.'b": 5}""", """$["a.'b"].c""",
Literal(2),
+ "VARIANT_PATH_TYPE_MISMATCH",
+ Map("path" -> "$[\"a.'b\"].c", "failedAt" -> "$[\"a.'b\"]",
+ "functionName" -> "`variant_insert`"))
+ checkInsertRecoverableError("""{"a.'\"b": 5}""", """$['a.\'"b'].c""",
Literal(2),
+ "VARIANT_PATH_TYPE_MISMATCH",
+ Map(
+ "path" -> """$['a.\'"b'].c""",
+ "failedAt" -> """$["a.'\"b"]""",
+ "functionName" -> "`variant_insert`"))
+ checkInsertRecoverableError("""{"a.\\nb": 5}""", """$['a.\\nb'].c""",
Literal(2),
Review Comment:
**Nit (P3):** `quoteKey` escapes control characters (`\b \f \n \r \t`, and
`< 0x20` as `\uXXXX`), and `renderVariantPath` uses it to build round-trippable
`failedAt`/`path` diagnostics. The parse-side decode of these escapes is now
well covered (including `\b`/`\f`/`\r`), but the round-trip rendering cases
here only exercise keys with a quote or a literal backslash -- none has an
actual control character together with a `.`/`[` that forces the bracket-render
branch. A regression that emitted a raw control char instead of its escape
would stop the rendered path from re-parsing to the same segments, with no
failing test. Worth adding one round-trip case whose key contains a control
char.
**Verification:**
- **Regression:** If quoteKey emits a raw control char instead of its
escape, the round-trip assertion fails.
--
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]
