Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release/0.22.3 #204

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Release/0.22.3 #204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
134e3be
Toggle some booleans in `Caster`
pondzix Mar 26, 2024
5a2e5a7
Prepare for 0.22.3 release
github-actions[bot] Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
Version 0.22.3 (2024-03-26)
---------------------------
Toggle some booleans in `Caster`

Version 0.22.2 (2024-03-25)
---------------------------
Upgrade dependencies
Expand Down
8 changes: 5 additions & 3 deletions modules/core/src/main/scala/com.snowplowanalytics/iglu.schemaddl/parquet/Caster.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,18 @@ object Caster {
.map { name =>
jsonObject.get(name) match {
case Some(Json.Null) => CastAccumulate[A](None, true)
case None => CastAccumulate[A](None, true)
case None => CastAccumulate[A](None, false)
case Some(json) => CastAccumulate(cast(caster, field, json).some, false)
}
}
.reduce(_ |+| _)

ca match {
case CastAccumulate(Some(Validated.Invalid(_)), true) if field.nullability.nullable =>
case CastAccumulate(Some(Validated.Invalid(_)), _) if field.nullability.nullable =>
// Shouldn't be an error? Regardless if any null/missing value observed,
// there is no valid value and there are some accessors providing invalid value
NamedValue(Field.normalizeName(field), caster.nullValue).validNel
case CastAccumulate(None, true) if field.nullability.nullable =>
case CastAccumulate(None, _) if field.nullability.nullable =>
NamedValue(Field.normalizeName(field), caster.nullValue).validNel
case CastAccumulate(None, true) =>
WrongType(Json.Null, field.fieldType).invalidNel
Expand Down
24 changes: 19 additions & 5 deletions modules/core/src/test/scala/com/snowplowanalytics/iglu/schemaddl/parquet/CasterSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,21 @@ import io.circe.literal._
import cats.data.NonEmptyList
import org.specs2.matcher.ValidatedMatchers._
import org.specs2.matcher.MatchResult

import com.snowplowanalytics.iglu.schemaddl.parquet.Type.Nullability.{Nullable, Required}
import com.snowplowanalytics.iglu.schemaddl.parquet.Type.DecimalPrecision.{Digits9, Digits18, Digits38}
import com.snowplowanalytics.iglu.schemaddl.parquet.Type.DecimalPrecision.{Digits18, Digits38, Digits9}
import com.snowplowanalytics.iglu.schemaddl.parquet.Caster.NamedValue
import com.snowplowanalytics.iglu.schemaddl.parquet.CastError._

import java.time.Instant

class CasterSpec extends org.specs2.Specification { def is = s2"""
cast transforms any primitive value $e1
cast transforms a UTC timestamp value $e2
cast transforms a non-UTC timestamp value $e3
cast transforms a date value $e4
cast transforms object with matching primitive fields $e5
cast transforms object with missing nullable field $e6
cast transforms object with missing required field $e18
cast transforms array values $e7
cast transforms nullable array values $e8
cast strips away undefined properties $e9
Expand Down Expand Up @@ -69,13 +71,13 @@ class CasterSpec extends org.specs2.Specification { def is = s2"""

def e2 = {
val input = json""""2022-02-02T01:02:03.123z""""
val expected = TimestampValue(java.sql.Timestamp.valueOf("2022-02-02 01:02:03.123"))
val expected = TimestampValue(Instant.parse("2022-02-02T01:02:03.123Z"))
testCast(Type.Timestamp, input, expected)
}

def e3 = {
val input = json""""2022-02-02T12:02:03.123+03:00""""
val expected = TimestampValue(java.sql.Timestamp.valueOf("2022-02-02 09:02:03.123"))
val expected = TimestampValue(Instant.parse("2022-02-02T09:02:03.123Z"))
testCast(Type.Timestamp, input, expected)
}

Expand Down Expand Up @@ -277,7 +279,8 @@ class CasterSpec extends org.specs2.Specification { def is = s2"""
json"""{"XYZ": 42, "xyz": "invalid"}""" -> StructValue(List(NamedValue("xyz", IntValue(42)))),
json"""{"xyz": null, "XYZ": "invalid"}""" -> StructValue(List(NamedValue("xyz", NullValue))),
json"""{"XYZ": null, "xyz": "invalid"}""" -> StructValue(List(NamedValue("xyz", NullValue))),
json"""{"XYZ": "invalid"}""" -> StructValue(List(NamedValue("xyz", NullValue))),
json"""{"XYZ": "invalid"}""" -> StructValue(List(NamedValue("xyz", NullValue))), //Should be error?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be error?

If line 280 and 281 are not errors, then I think it's ok for this one to also be not an error.

But I'm not strongly opinionated on this one, especially as it's a really weird edge case scenario, which we may never see in production. The important thing is: the loader doesn't break. And we've achieved that.

json"""{"xyz": "invalid", "XYZ": "invalid"}""" -> StructValue(List(NamedValue("xyz", NullValue))), // This one is null now! Should be error?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be error?

I think yes.

)
.map { case (json, expected) =>
testCast(inputField, json, expected)
Expand All @@ -293,4 +296,15 @@ class CasterSpec extends org.specs2.Specification { def is = s2"""
val expected = NonEmptyList.one(WrongType(Json.Null, Type.String))
Caster.cast(caster, Field("top", fieldType, Nullable), inputJson) must beInvalid(expected)
}

def e18 = {
val inputJson = json"""{"bar": true}"""
val inputField = Type.Struct(List(
Field("foo", Type.Integer, Required),
Field("bar", Type.Boolean, Required)))

val expected = NonEmptyList.one(MissingInValue("foo", json"""{"bar": true}"""))
Caster.cast(caster, Field("top", inputField, Nullable), inputJson) must beInvalid(expected)
}

}
4 changes: 2 additions & 2 deletions .../core/src/test/scala/com/snowplowanalytics/iglu/schemaddl/parquet/ExampleFieldValue.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ object ExampleFieldValue {
case class LongValue(value: Long) extends ExampleFieldValue
case class DoubleValue(value: Double) extends ExampleFieldValue
case class DecimalValue(value: BigDecimal, precision: Type.DecimalPrecision) extends ExampleFieldValue
case class TimestampValue(value: java.sql.Timestamp) extends ExampleFieldValue
case class TimestampValue(value: Instant) extends ExampleFieldValue
case class DateValue(value: java.sql.Date) extends ExampleFieldValue
case class StructValue(values: List[Caster.NamedValue[ExampleFieldValue]]) extends ExampleFieldValue
case class ArrayValue(values: List[ExampleFieldValue]) extends ExampleFieldValue
Expand All @@ -44,7 +44,7 @@ object ExampleFieldValue {
def decimalValue(unscaled: BigInt, details: Type.Decimal): ExampleFieldValue =
DecimalValue(BigDecimal(unscaled, details.scale), details.precision)
def dateValue(v: LocalDate): ExampleFieldValue = DateValue(java.sql.Date.valueOf(v))
def timestampValue(v: Instant): ExampleFieldValue = TimestampValue(java.sql.Timestamp.from(v))
def timestampValue(v: Instant): ExampleFieldValue = TimestampValue(v)
def structValue(vs: List[Caster.NamedValue[ExampleFieldValue]]): ExampleFieldValue = StructValue(vs)
def arrayValue(vs: List[ExampleFieldValue]): ExampleFieldValue = ArrayValue(vs)
}
Expand Down