From d754874aead730f1df6d19cda53e3c11b5092c1f Mon Sep 17 00:00:00 2001 From: Oguzhan Unlu Date: Wed, 14 Feb 2024 17:18:24 +0300 Subject: [PATCH] Fix treating no type change as incompatible (close #197) --- .../iglu.schemaddl/redshift/ShredModel.scala | 32 ++--- .../schemaddl/redshift/ShredModelSpec.scala | 114 +++++++++++++++++- 2 files changed, 128 insertions(+), 18 deletions(-) diff --git a/modules/core/src/main/scala/com.snowplowanalytics/iglu.schemaddl/redshift/ShredModel.scala b/modules/core/src/main/scala/com.snowplowanalytics/iglu.schemaddl/redshift/ShredModel.scala index 53e9b534..536028e6 100644 --- a/modules/core/src/main/scala/com.snowplowanalytics/iglu.schemaddl/redshift/ShredModel.scala +++ b/modules/core/src/main/scala/com.snowplowanalytics/iglu.schemaddl/redshift/ShredModel.scala @@ -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 @@ -150,7 +165,6 @@ 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 @@ -158,24 +172,11 @@ object ShredModel { 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 @@ -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 } }) diff --git a/modules/core/src/test/scala/com/snowplowanalytics/iglu/schemaddl/redshift/ShredModelSpec.scala b/modules/core/src/test/scala/com/snowplowanalytics/iglu/schemaddl/redshift/ShredModelSpec.scala index 8534e042..a3a7fda9 100644 --- a/modules/core/src/test/scala/com/snowplowanalytics/iglu/schemaddl/redshift/ShredModelSpec.scala +++ b/modules/core/src/test/scala/com/snowplowanalytics/iglu/schemaddl/redshift/ShredModelSpec.scala @@ -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"""{ @@ -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, @@ -615,7 +726,6 @@ class ShredModelSpec extends Specification { |Incompatible types in column foo old RedshiftVarchar(20) new RedshiftDouble""".stripMargin ) } - } }