Skip to content

Commit

Permalink
Fix treating no type change as incompatible (close #197)
Browse files Browse the repository at this point in the history
  • Loading branch information
@oguzhanunlu
oguzhanunlu committed Feb 14, 2024
1 parent 4f63818 commit d754874
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 18 deletions.
32 changes: 16 additions & 16 deletions modules/core/src/main/scala/com.snowplowanalytics/iglu.schemaddl/redshift/ShredModel.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,23 @@ object ShredModel {
case ColumnType.RedshiftVarchar(oldSize) if newSize <= oldSize => NoChanges.asRight
case _ => IncompatibleTypes(oldCol, newCol).asLeft.toEitherNel
}
case ColumnType.RedshiftDecimal(new_precision, new_scale) => oldType match {
case ColumnType.RedshiftDouble => NoChanges.asRight
case ColumnType.RedshiftVarchar(oldSize) if oldSize >= new_precision.getOrElse(18) => NoChanges.asRight
case ColumnType.RedshiftDecimal(old_precision, old_scale)
if old_precision.getOrElse(18) >= new_precision.getOrElse(18) &
old_scale.getOrElse(0) >= new_scale.getOrElse(0) => NoChanges.asRight
case _ => IncompatibleTypes(oldCol, newCol).asLeft.toEitherNel
}
case ColumnType.RedshiftChar(newSize) => oldType match {
case ColumnType.RedshiftVarchar(oldSize) if oldSize >= newSize => NoChanges.asRight
case ColumnType.RedshiftChar(oldSize) if oldSize >= newSize => NoChanges.asRight
case _ => IncompatibleTypes(oldCol, newCol).asLeft.toEitherNel
}
// above are cases where newType == oldType does not always result in NoChanges
case _ if newType == oldType => NoChanges.asRight
// below are cases where newType == oldType always result in NoChanges
case ColumnType.RedshiftSmallInt => oldType match {
case ColumnType.RedshiftSmallInt => NoChanges.asRight
case ColumnType.RedshiftInteger => NoChanges.asRight
case ColumnType.RedshiftBigInt => NoChanges.asRight
case ColumnType.RedshiftDouble => NoChanges.asRight
Expand All @@ -150,32 +165,18 @@ object ShredModel {
case _ => IncompatibleTypes(oldCol, newCol).asLeft.toEitherNel
}
case ColumnType.RedshiftInteger => oldType match {
case ColumnType.RedshiftInteger => NoChanges.asRight
case ColumnType.RedshiftBigInt => NoChanges.asRight
case ColumnType.RedshiftDouble => NoChanges.asRight
case ColumnType.RedshiftVarchar(oldSize) if oldSize >= 10 => NoChanges.asRight
case ColumnType.RedshiftDecimal(precision, scale) if precision.getOrElse(18) - scale.getOrElse(0) >= 10 => NoChanges.asRight
case _ => IncompatibleTypes(oldCol, newCol).asLeft.toEitherNel
}
case ColumnType.RedshiftBigInt => oldType match {
case ColumnType.RedshiftBigInt => NoChanges.asRight
case ColumnType.RedshiftDouble => NoChanges.asRight
case ColumnType.RedshiftVarchar(oldSize) if oldSize >= 19 => NoChanges.asRight
case ColumnType.RedshiftDecimal(precision, scale) if precision.getOrElse(18) - scale.getOrElse(0) >= 19 => NoChanges.asRight
case _ => IncompatibleTypes(oldCol, newCol).asLeft.toEitherNel
}
case ColumnType.RedshiftDecimal(new_precision, new_scale) => oldType match {
case ColumnType.RedshiftDouble => NoChanges.asRight
case ColumnType.RedshiftVarchar(oldSize) if oldSize >= new_precision.getOrElse(18) => NoChanges.asRight
case ColumnType.RedshiftDecimal(old_precision, old_scale)
if old_precision.getOrElse(18) >= new_precision.getOrElse(18) &
old_scale.getOrElse(0) >= new_scale.getOrElse(0) => NoChanges.asRight
case _ => IncompatibleTypes(oldCol, newCol).asLeft.toEitherNel
}
case ColumnType.RedshiftChar(newSize) => oldType match {
case ColumnType.RedshiftVarchar(oldSize) if oldSize >= newSize => NoChanges.asRight
case _ => IncompatibleTypes(oldCol, newCol).asLeft.toEitherNel
}
case ColumnType.RedshiftDate => oldType match {
case ColumnType.RedshiftVarchar(oldSize) if oldSize >= 10 => NoChanges.asRight
case _ => IncompatibleTypes(oldCol, newCol).asLeft.toEitherNel
Expand All @@ -189,7 +190,6 @@ object ShredModel {
case ColumnType.RedshiftVarchar(oldSize) if oldSize >= 5 => NoChanges.asRight
case _ => IncompatibleTypes(oldCol, newCol).asLeft.toEitherNel
}
case _ if newType == oldType => NoChanges.asRight
case _ => IncompatibleTypes(oldCol, newCol).asLeft.toEitherNel
}
})
Expand Down
114 changes: 112 additions & 2 deletions ...es/core/src/test/scala/com/snowplowanalytics/iglu/schemaddl/redshift/ShredModelSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,118 @@ class ShredModelSpec extends Specification {
}

"model migrations" should {
"should merge with no type change (date-time)" in {
val s1 = ShredModel.good(dummyKey,
json"""{
"type": "object",
"properties": {
"foo": {
"type": "string",
"format": "date-time"
}}
}""".schema)
val s2 = ShredModel.good(dummyKey1,
json"""{
"type": "object",
"properties": {
"foo": {
"type": "string",
"format": "date-time"
}}
}""".schema)
s1.merge(s2).toTestString must beRight
}

"should merge with no type change (date)" in {
val s1 = ShredModel.good(dummyKey,
json"""{
"type": "object",
"properties": {
"foo": {
"type": "string",
"format": "date"
}}
}""".schema)
val s2 = ShredModel.good(dummyKey1,
json"""{
"type": "object",
"properties": {
"foo": {
"type": "string",
"format": "date"
}}
}""".schema)
s1.merge(s2).toTestString must beRight
}

"should merge with no type change (boolean)" in {
val s1 = ShredModel.good(dummyKey,
json"""{
"type": "object",
"properties": {
"foo": {
"type": "boolean"
}}
}""".schema)
val s2 = ShredModel.good(dummyKey1,
json"""{
"type": "object",
"properties": {
"foo": {
"type": "boolean"
}}
}""".schema)
s1.merge(s2).toTestString must beRight
}

"should merge with no type change (char to smaller size)" in {
val s1 = ShredModel.good(dummyKey,
json"""{
"type": "object",
"properties": {
"foo": {
"type": "string",
"minLength": "12",
"maxLength": "12"
}}
}""".schema)
val s2 = ShredModel.good(dummyKey1,
json"""{
"type": "object",
"properties": {
"foo": {
"type": "string",
"minLength": "10",
"maxLength": "10"
}}
}""".schema)
s1.merge(s2).toTestString must beRight
}

"should not merge with no type change (char to larger size)" in {
val s1 = ShredModel.good(dummyKey,
json"""{
"type": "object",
"properties": {
"foo": {
"type": "string",
"minLength": "12",
"maxLength": "12"
}}
}""".schema)
val s2 = ShredModel.good(dummyKey1,
json"""{
"type": "object",
"properties": {
"foo": {
"type": "string",
"minLength": "15",
"maxLength": "15"
}}
}""".schema)
s1.merge(s2).toTestString must beLeft
}

"should merge with varchar widening" in {
val s1 = ShredModel.good(dummyKey,
json"""{
Expand Down Expand Up @@ -294,7 +406,6 @@ class ShredModelSpec extends Specification {
|Making required column nullable foo""".stripMargin
)
}


"should make a ignore varchar narrowing" in {
val s1 = ShredModel.good(dummyKey,
Expand Down Expand Up @@ -615,7 +726,6 @@ class ShredModelSpec extends Specification {
|Incompatible types in column foo old RedshiftVarchar(20) new RedshiftDouble""".stripMargin
)
}

}
}

Expand Down

0 comments on commit d754874

Please sign in to comment.