cloud-fan commented on code in PR #57827:
URL: https://github.com/apache/spark/pull/57827#discussion_r3936021194
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/variant/variantExpressions.scala:
##########
@@ -1851,6 +1905,82 @@ object VariantExplode {
case _ => Nil
}
}
+
+ private case class ExplodeEntry(path: String, pos: Int, key: UTF8String,
value: Variant)
+
+ def variantExplodeRecursive(
+ input: VariantVal,
+ isNull: Boolean): Iterable[InternalRow] = {
+ if (isNull) {
+ return Iterable.empty
+ }
+
+ new Iterable[InternalRow] {
+ override def iterator: Iterator[InternalRow] = {
Review Comment:
**Non-blocking (P2):** For interpreted variant_explode_outer, GenerateExec
first probes outputRows.iterator.isEmpty and then requests another iterator for
consumption. This Iterable rebuilds and repopulates the complete root stack on
each request, so every non-empty row repeats O(number of direct children) path
creation and allocation. Could this return a one-shot Iterator or IterableOnce
whose initialized stack is shared by the probe and consumption?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/variant/variantExpressions.scala:
##########
@@ -1851,6 +1905,82 @@ object VariantExplode {
case _ => Nil
}
}
+
+ private case class ExplodeEntry(path: String, pos: Int, key: UTF8String,
value: Variant)
+
+ def variantExplodeRecursive(
+ input: VariantVal,
+ isNull: Boolean): Iterable[InternalRow] = {
+ if (isNull) {
+ return Iterable.empty
+ }
+
+ new Iterable[InternalRow] {
+ override def iterator: Iterator[InternalRow] = {
+ val stack = new ArrayDeque[ExplodeEntry]()
+ pushChildren(new Variant(input.getValue, input.getMetadata), "$",
stack)
+ new Iterator[InternalRow] {
+ override def hasNext: Boolean = !stack.isEmpty
+
+ override def next(): InternalRow = {
+ val entry = stack.pop()
+ pushChildren(entry.value, entry.path, stack)
+ InternalRow(
+ UTF8String.fromString(entry.path),
+ entry.pos,
+ entry.key,
+ new VariantVal(entry.value.getValue, entry.value.getMetadata))
+ }
+ }
+ }
+ }
+ }
+
+ private def pushChildren(
+ v: Variant,
+ parentPath: String,
+ stack: ArrayDeque[ExplodeEntry]): Unit = {
+ v.getType match {
+ case Type.OBJECT =>
+ for (i <- v.objectSize() - 1 to 0 by -1) {
+ val field = v.getFieldAtIndex(i)
+ stack.push(ExplodeEntry(
+ appendObjectPath(parentPath, field.key),
+ i,
+ UTF8String.fromString(field.key),
+ field.value))
+ }
+ case Type.ARRAY =>
+ for (i <- v.arraySize() - 1 to 0 by -1) {
+ stack.push(ExplodeEntry(
+ s"$parentPath[$i]",
+ i,
+ null,
+ v.getElementAtIndex(i)))
+ }
+ case _ =>
+ }
+ }
+
+ // Appends `key` to `parentPath`: dot notation for dot-safe keys, else
`['...']` with `\` and
+ // `'` escaped.
+ private def appendObjectPath(parentPath: String, key: String): String = {
+ if (isDotSafeKey(key)) {
+ s"$parentPath.$key"
+ } else {
+ s"$parentPath['${key.replace("\\", "\\\\").replace("'", "\\'")}']"
Review Comment:
**Blocking (P1):** The quoted-key renderer and VariantPathParser use
different escape rules. Apostrophe paths are rejected, doubled backslashes
address a different key, and control characters such as tabs remain raw even
though this column is documented as JSONPath. Could we give appendObjectPath
and VariantPathParser one reversible escape contract and add tests that feed
these emitted paths back into variant_get?
**Recommended change:** Use one reversible quoted-key escape contract for
recursive path generation and VariantPathParser, and verify it through
variant_get round trips.
**Why this works:** Encode quotes, backslashes, and control characters in
appendObjectPath with syntax that VariantPathParser recognizes and decodes back
to the original object key.
**Scope:** Catalyst Variant path rendering/parsing and focused VariantSuite
round-trip coverage.
**Compatibility:** Preserve existing simple dot and bracket paths while
making every newly emitted quoted path consumable; account for callers that
currently treat backslashes literally.
**Risks:** Changing parser escape semantics could reinterpret existing
manually supplied bracket paths containing literal backslashes.
**Constraints:** Support the full valid Variant object-key domain, including
both quote characters, backslashes, and controls. Keep the emitted path and
variant_get consumer on the same representation contract.
**Success:** For every valid object key, feeding a recursive explode row's
path to variant_get retrieves that row's value without a parse error or key
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]
