Skip to content

Commit

Permalink
[query] support importing empty JSON objects (#14202)
Browse files Browse the repository at this point in the history
@patrick-schultz I'm not sure if this makes sense or not, but I observed
it while working on something else. It seems weird but acceptable to
import an empty dictionary as any struct. Does this seem reasonable to
you? How have we avoided this bug for so long?

I'm not familiar enough with this code to know how to simply reproduce
the bug and add a corresponding test. Thoughts?
  • Loading branch information
danking authored Feb 3, 2024
1 parent d4679eb commit 0b92923
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 4 deletions.
10 changes: 7 additions & 3 deletions hail/src/main/scala/is/hail/expr/AnnotationImpex.scala
Original file line number Diff line number Diff line change
Expand Up @@ -263,11 +263,15 @@ object JSONAnnotationImpex {
if (t.size == 0)
Annotation.empty
else {
val annotationSize =
if (padNulls) t.size
else jfields.map { case (name, _) =>
val annotationSize = if (padNulls) {
t.size
} else if (jfields.size == 0) {
0
} else {
jfields.map { case (name, _) =>
t.selfField(name).map(_.index).getOrElse(-1)
}.max + 1
}
val a = Array.fill[Any](annotationSize)(null)

for ((name, jv2) <- jfields) {
Expand Down
25 changes: 24 additions & 1 deletion hail/src/test/scala/is/hail/methods/ExprSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ import is.hail.check.Prop._
import is.hail.check.Properties
import is.hail.expr._
import is.hail.expr.ir.IRParser
import is.hail.types.virtual.{TInt32, Type}
import is.hail.types.virtual._
import is.hail.utils.StringEscapeUtils._

import org.json4s._
import org.json4s.jackson.JsonMethods._
import org.testng.annotations.Test
import org.apache.spark.sql.Row

class ExprSuite extends HailSuite {

Expand Down Expand Up @@ -70,6 +71,28 @@ class ExprSuite extends HailSuite {
p.check()
}

@Test def testImportEmptyJSONObjectAsStruct(): Unit =
assert(JSONAnnotationImpex.importAnnotation(parse("{}"), TStruct()) == Row())

@Test def testExportEmptyJSONObjectAsStruct(): Unit =
assert(compact(render(JSONAnnotationImpex.exportAnnotation(Row(), TStruct()))) == "{}")

@Test def testRoundTripEmptyJSONObject(): Unit = {
val actual = JSONAnnotationImpex.exportAnnotation(
JSONAnnotationImpex.importAnnotation(parse("{}"), TStruct()),
TStruct(),
)
assert(compact(render(actual)) == "{}")
}

@Test def testRoundTripEmptyStruct(): Unit = {
val actual = JSONAnnotationImpex.importAnnotation(
JSONAnnotationImpex.exportAnnotation(Row(), TStruct()),
TStruct(),
)
assert(actual == Row())
}

@Test def testImpexes(): Unit = {

val g = for {
Expand Down

0 comments on commit 0b92923

Please sign in to comment.