Skip to content

Commit 574db49

Browse files
author
Michel Davit
committed
Fix literal string and json string disambiguation
1 parent b06a26d commit 574db49

File tree

5 files changed

+57
-14
lines changed

5 files changed

+57
-14
lines changed

integration/src/test/scala/com/spotify/scio/bigquery/TypedBigQueryIT.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,9 @@ object TypedBigQueryIT {
8484
.oneOf(
8585
alphaLowerStr.flatMap(str => arbInt.arbitrary.map(num => s"""{"$str":$num}""")),
8686
alphaLowerStr.flatMap(str => arbInt.arbitrary.map(num => s"""["$str",$num]""")),
87-
alphaLowerStr.map(str => s"\"$str\"")
88-
// arbInt.arbitrary.map(_.toString), TableRow serialization converts to string literal
89-
// arbBool.arbitrary.map(_.toString), TableRow serialization converts to string literal
87+
alphaLowerStr.map(str => s"\"$str\""),
88+
arbInt.arbitrary.map(_.toString),
89+
arbBool.arbitrary.map(_.toString)
9090
// Gen.const("null"), null json literal is lost, interpreted as missing field
9191
)
9292
.map(wkt => Json(wkt))
@@ -112,7 +112,7 @@ object TypedBigQueryIT {
112112
private val tableRowTable = table("records_tablerow")
113113
private val avroTable = table("records_avro")
114114

115-
private val records = Gen.listOfN(5, recordGen).sample.get
115+
private val records = Gen.listOfN(100, recordGen).sample.get
116116
private val options = PipelineOptionsFactory
117117
.fromArgs(
118118
"--project=data-integration-test",

scio-google-cloud-platform/src/main/scala/com/spotify/scio/bigquery/syntax/TableRowSyntax.scala

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,12 @@ object TableRowOps {
126126
case null => Json(null)
127127
case x: java.lang.Number => Json(x)
128128
case x: java.lang.Boolean => Json(x)
129+
case x: java.lang.String => Json(x) // also handles json string
129130
// object
130131
case x: java.util.Map[_, _] => Json(x)
131132
// array
132133
case x: java.util.List[_] => Json(x)
133-
// json string / literal
134-
case x: String => Json(x)
135-
case _ => throw new UnsupportedOperationException("Cannot convert to json: " + value)
134+
case _ => throw new UnsupportedOperationException("Cannot convert to json: " + value)
136135
}
137136

138137
def bignumeric(value: AnyRef): BigNumeric = value match {

scio-google-cloud-platform/src/main/scala/com/spotify/scio/bigquery/types/package.scala

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import org.typelevel.scalaccompat.annotation.nowarn
2727

2828
import java.math.MathContext
2929
import java.nio.ByteBuffer
30-
import scala.annotation.StaticAnnotation
30+
import scala.annotation.{unused, StaticAnnotation}
3131
import scala.util.Try
3232

3333
package object types {
@@ -77,12 +77,15 @@ package object types {
7777
.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
7878
.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS);
7979

80+
// force to use the apply(AnyRef)
81+
@unused private def apply(wkt: String): Json = new Json(wkt)
82+
8083
def apply(value: AnyRef): Json = value match {
8184
case str: String if Try(mapper.readTree(str)).isSuccess =>
8285
// string formatted json vs string literal
83-
Json(str)
86+
new Json(str)
8487
case _ =>
85-
Json(mapper.writeValueAsString(value))
88+
new Json(mapper.writeValueAsString(value))
8689
}
8790
def parse(json: Json): AnyRef = mapper.readValue(json.wkt, classOf[Object])
8891
}

scio-google-cloud-platform/src/test/scala/com/spotify/scio/bigquery/types/ConverterProviderSpec.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,7 @@ final class ConverterProviderSpec
7777
property("round trip required primitive types") {
7878
forAll { r1: Required =>
7979
val r2 = BigQueryType.fromTableRow[Required](BigQueryType.toTableRow[Required](r1))
80-
val res = EqDerivation[Required].eqv(r1, r2)
81-
res shouldBe true
80+
EqDerivation[Required].eqv(r1, r2) shouldBe true
8281
}
8382
}
8483

scio-google-cloud-platform/src/test/scala/com/spotify/scio/bigquery/types/ConverterProviderTest.scala

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,19 @@ class ConverterProviderTest extends AnyFlatSpec with Matchers {
5353
it should "handle required json type" in {
5454
// JSON object
5555
{
56-
val wkt = """{"name":"Alice","age":30,"address":{"street":"Broadway","city":"New York"}}"""
56+
val wkt =
57+
"""{"name":"Alice","age":30,"job":null,"address":{"street":"Broadway","city":"New York"}}"""
5758
val parsed = Map(
5859
"name" -> "Alice",
5960
"age" -> 30,
61+
"job" -> null,
6062
"address" -> Map(
6163
"street" -> "Broadway",
6264
"city" -> "New York"
6365
).asJava
6466
).asJava
6567

68+
RequiredJson.fromTableRow(TableRow("a" -> wkt)) shouldBe RequiredJson(Json(wkt))
6669
RequiredJson.fromTableRow(TableRow("a" -> parsed)) shouldBe RequiredJson(Json(wkt))
6770
BigQueryType.toTableRow[RequiredJson](RequiredJson(Json(wkt))) shouldBe TableRow(
6871
"a" -> parsed
@@ -71,22 +74,61 @@ class ConverterProviderTest extends AnyFlatSpec with Matchers {
7174

7275
// JSON array
7376
{
74-
val wkt = """["Alice",30,{"street":"Broadway","city":"New York"}]"""
77+
val wkt = """["Alice",30,null,{"street":"Broadway","city":"New York"}]"""
7578
val parsed = List(
7679
"Alice",
7780
30,
81+
null,
7882
Map(
7983
"street" -> "Broadway",
8084
"city" -> "New York"
8185
).asJava
8286
).asJava
8387

88+
RequiredJson.fromTableRow(TableRow("a" -> wkt)) shouldBe RequiredJson(Json(wkt))
8489
RequiredJson.fromTableRow(TableRow("a" -> parsed)) shouldBe RequiredJson(Json(wkt))
8590
BigQueryType.toTableRow[RequiredJson](RequiredJson(Json(wkt))) shouldBe TableRow(
8691
"a" -> parsed
8792
)
8893
}
8994

95+
// JSON string literal
96+
{
97+
val wkt = "\"Hello world!\""
98+
val parsed = "Hello world!"
99+
100+
RequiredJson.fromTableRow(TableRow("a" -> wkt)) shouldBe RequiredJson(Json(wkt))
101+
RequiredJson.fromTableRow(TableRow("a" -> parsed)) shouldBe RequiredJson(Json(wkt))
102+
BigQueryType.toTableRow[RequiredJson](RequiredJson(Json(wkt))) shouldBe TableRow(
103+
"a" -> parsed
104+
)
105+
}
106+
107+
// JSON boolean literal
108+
{
109+
val wkt = "false"
110+
val parsed = false
111+
112+
RequiredJson.fromTableRow(TableRow("a" -> wkt)) shouldBe RequiredJson(Json(wkt))
113+
RequiredJson.fromTableRow(TableRow("a" -> parsed)) shouldBe RequiredJson(Json(wkt))
114+
BigQueryType.toTableRow[RequiredJson](RequiredJson(Json(wkt))) shouldBe TableRow(
115+
"a" -> parsed
116+
)
117+
}
118+
119+
// JSON number literal
120+
{
121+
val wkt = "42"
122+
val parsed = 42
123+
124+
RequiredJson.fromTableRow(TableRow("a" -> wkt)) shouldBe RequiredJson(Json(wkt))
125+
RequiredJson.fromTableRow(TableRow("a" -> parsed)) shouldBe RequiredJson(Json(wkt))
126+
BigQueryType.toTableRow[RequiredJson](RequiredJson(Json(wkt))) shouldBe TableRow(
127+
"a" -> parsed
128+
)
129+
}
130+
131+
// JSON null literal is ambiguous with NULLABLE column
90132
}
91133

92134
it should "handle required big numeric type" in {

0 commit comments

Comments
 (0)