bojana-db commented on code in PR #57827:
URL: https://github.com/apache/spark/pull/57827#discussion_r3813497946
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/variant/variantExpressions.scala:
##########
@@ -1603,6 +1659,74 @@ 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 _ =>
+ }
+ }
+
+ private def appendObjectPath(parentPath: String, key: String): String = {
+ if (key.nonEmpty && !key.contains('.') && !key.contains('[')) {
+ s"$parentPath.$key"
+ } else if (!key.contains('"')) {
+ s"""$parentPath["$key"]"""
+ } else if (!key.contains('\'')) {
+ s"$parentPath['$key']"
+ } else {
+ s"""$parentPath["${StringEscapeUtils.escapeJson(key)}"]"""
Review Comment:
Yeah, that makes sense. Do you know whether we are planning to add
escape-character support to `VariantPathParser`? That would let us emit paths
that can always be passed back to `variant_get`.
The other option is to emit `NULL` for keys that cannot be represented, but
I do not like that behavior. `variant_get(..., path = `NULL`)` returns `NULL`,
so users could unknowingly fail to retrieve values or exclude rows derived from
those paths. In my opinion, explicitly failing for an unrepresentable path
would be more truthful.
--
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]
