use {tag, value} format for SQLizing advanced JSON queries (#9321)

* literal variant test case for compiled query * nicer error reporting of positional args in Fragments * a failing advanced case for variant query * use proper path * also check nested number roundtrip * add changelog CHANGELOG_BEGIN - [HTTP JSON API] Range queries within variant data would not return matching data when using the PostgreSQL backend with JSON API; this is fixed. See `issue #9321 <https://github.com/digital-asset/daml/pull/9321>`__. CHANGELOG_END
digital-asset · Apr 6, 2021 · fd63cf0 · fd63cf0
1 parent 631db44
commit fd63cf0
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 18 deletions.
diff --git a/ledger-service/http-json/src/main/scala/com/digitalasset/http/query/ValuePredicate.scala b/ledger-service/http-json/src/main/scala/com/digitalasset/http/query/ValuePredicate.scala
@@ -127,13 +127,14 @@ sealed abstract class ValuePredicate extends Product with Serializable {
           goObject(path, cqs, qs.length)
 
         case VariantMatch((dc, q)) =>
-          val Rec(vraw, v_==, v_@>) = go(path objectAt dc, q) // TODO SC exercise this path
-          // @> is safe because in a variant-typed context, all JsObjects
-          // have exactly two keys
+          val Rec(vraw, v_==, v_@>) = go(path objectAt JsonVariant.valueKey, q)
+          // @> induction is safe because in a variant-typed context, all JsObjects
+          // have exactly two keys. @> is conjoined with raw so we use it to add
+          // the tag check
           Rec(
             vraw,
-            v_== map (jv => JsonVariant(dc, jv)),
-            v_@> map (jv => JsonVariant(dc, jv)),
+            v_== map (JsonVariant(dc, _)),
+            v_@> map (JsonVariant(dc, _)) orElse Some(JsonVariant withoutValue dc),
           )
 
         case OptionalMatch(None) =>

diff --git a/ledger-service/http-json/src/test/scala/com/digitalasset/http/query/ValuePredicateTest.scala b/ledger-service/http-json/src/test/scala/com/digitalasset/http/query/ValuePredicateTest.scala
@@ -9,7 +9,7 @@ import com.daml.lf.data.{Decimal, ImmArray, Numeric, Ref, SortedLookupList, Time
 import ImmArray.ImmArraySeq
 import com.daml.lf.iface
 import com.daml.lf.value.{Value => V}
-import com.daml.lf.value.test.TypedValueGenerators.{genAddendNoListMap, ValueAddend => VA}
+import com.daml.lf.value.test.TypedValueGenerators.{genAddendNoListMap, RNil, ValueAddend => VA}
 
 import org.scalacheck.{Arbitrary, Gen}
 import org.scalactic.source
@@ -18,7 +18,7 @@ import org.scalatestplus.scalacheck.ScalaCheckDrivenPropertyChecks
 import org.scalatest.Inside
 import org.scalatest.matchers.should.Matchers
 import org.scalatest.wordspec.AnyWordSpec
-import scalaz.Order
+import scalaz.{\/, Order}
 import spray.json._
 
 @SuppressWarnings(Array("org.wartremover.warts.NonUnitStatements"))
@@ -38,12 +38,24 @@ class ValuePredicateTest
     case a0 @ V.ContractId.V0(_) => a0
   })
 
+  import Ref.QualifiedName.{assertFromString => qn}
+
+  private[this] val dummyPackageId = Ref.PackageId assertFromString "dummy-package-id"
+  private[this] def eitherT = {
+    import shapeless.syntax.singleton._
+    val sig = Symbol("Left") ->> VA.int64 ::
+      Symbol("Right") ->> VA.text ::
+      RNil
+    val id = Ref.Identifier(dummyPackageId, qn("Foo:Either"))
+    (id, VA.variant(id, sig))
+  }
   private[this] val dummyId = Ref.Identifier(
-    Ref.PackageId assertFromString "dummy-package-id",
-    Ref.QualifiedName assertFromString "Foo:Bar",
+    dummyPackageId,
+    qn("Foo:Bar"),
   )
   private[this] val dummyFieldName = Ref.Name assertFromString "foo"
   private[this] val dummyTypeCon = iface.TypeCon(iface.TypeConName(dummyId), ImmArraySeq.empty)
+  private[this] val (eitherId, (eitherDDT, eitherVA)) = eitherT
   private[this] def valueAndTypeInObject(
       v: V[Cid],
       ty: iface.Type,
@@ -52,7 +64,8 @@ class ValuePredicateTest
   private[this] def typeInObject(ty: iface.Type): ValuePredicate.TypeLookup =
     Map(
       dummyId -> iface
-        .DefDataType(ImmArraySeq.empty, iface.Record(ImmArraySeq((dummyFieldName, ty))))
+        .DefDataType(ImmArraySeq.empty, iface.Record(ImmArraySeq((dummyFieldName, ty)))),
+      eitherId -> eitherDDT,
     ).lift
 
   "fromJsObject" should {
@@ -232,6 +245,16 @@ class ValuePredicateTest
           VA.int64,
           sql"payload->${"foo": String} <= ${JsNumber(42): JsValue}::jsonb AND payload @> ${JsObject(): JsValue}::jsonb",
         ),
+        (
+          """{"tag": "Left", "value": "42"}""",
+          eitherVA,
+          sql"payload = ${"""{"foo": {"tag": "Left", "value": 42}}""".parseJson}::jsonb",
+        ),
+        (
+          """{"tag": "Left", "value": {"%lte": 42}}""",
+          eitherVA,
+          sql"payload->${"foo"}->${"value"} <= ${"42".parseJson}::jsonb AND payload @> ${"""{"foo": {"tag": "Left"}}""".parseJson}::jsonb",
+        ),
       )
     }
 
@@ -244,16 +267,19 @@ class ValuePredicateTest
       )
       val frag = vp.toSqlWhereClause
       frag.toString should ===(sql.toString)
-      import language.reflectiveCalls
-      frag.asInstanceOf[{ def elems: FragmentElems }].elems should ===(
-        sql.asInstanceOf[{ def elems: FragmentElems }].elems
-      )
+      fragmentElems(frag) should ===(fragmentElems(sql))
     }
   }
 }
 
 object ValuePredicateTest {
-  import cats.data.Chain, doobie.util.fragment.Elem
+  import cats.data.Chain, doobie.util.fragment.{Elem, Fragment}
 
-  private type FragmentElems = Chain[Elem]
+  private def fragmentElems(frag: Fragment): Chain[Any \/ Option[Any]] = {
+    import language.reflectiveCalls, Elem.{Arg, Opt}
+    frag.asInstanceOf[{ def elems: Chain[Elem] }].elems.map {
+      case Arg(a, _) => \/.left(a)
+      case Opt(o, _) => \/.right(o)
+    }
+  }
 }
diff --git a/...ervice/lf-value-json/src/main/scala/com/digitalasset/daml/lf/value/json/JsonVariant.scala b/...ervice/lf-value-json/src/main/scala/com/digitalasset/daml/lf/value/json/JsonVariant.scala
@@ -3,14 +3,21 @@
 
 package com.daml.lf.value.json
 
+import com.daml.lf.data.Ref
 import spray.json.{JsObject, JsString, JsValue}
 
 object JsonVariant {
   def apply(tag: String, body: JsValue): JsObject =
-    JsObject("tag" -> JsString(tag), "value" -> body)
+    JsObject(tagKey -> JsString(tag), valueKey -> body)
+
+  def withoutValue(tag: String): JsObject =
+    JsObject(tagKey -> JsString(tag))
+
+  val tagKey = "tag"
+  val valueKey: Ref.Name = Ref.Name assertFromString "value"
 
   def unapply(o: JsObject): Option[(String, JsValue)] =
-    (o.fields.size, o.fields.get("tag"), o.fields.get("value")) match {
+    (o.fields.size, o.fields.get(tagKey), o.fields.get(valueKey)) match {
       case (2, Some(JsString(tag)), Some(nv)) => Some((tag, nv))
       case _ => None
     }