Make mysql YEAR type behave like an integer rather than a string (#25101

) * Make mysql YEAR type behave like an integer rather than a string * sanity * sanity
airbytehq · Apr 26, 2023 · 6b32870 · 6b32870
1 parent 4164cd9
commit 6b32870
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 28 deletions.
diff --git a/...nnectors/source-mysql/src/main/java/io/airbyte/integrations/source/mysql/MySqlSource.java b/...nnectors/source-mysql/src/main/java/io/airbyte/integrations/source/mysql/MySqlSource.java
@@ -191,7 +191,7 @@ public JsonNode toDatabaseConfig(final JsonNode config) {
     jdbcUrl.append("&tinyInt1isBit=true");
     // ensure the return year value is a Date; see the rationale
     // in the setJsonField method in MySqlSourceOperations.java
-    jdbcUrl.append("&yearIsDateType=true");
+    jdbcUrl.append("&yearIsDateType=false");
     if (config.get(JdbcUtils.JDBC_URL_PARAMS_KEY) != null && !config.get(JdbcUtils.JDBC_URL_PARAMS_KEY).asText().isEmpty()) {
       jdbcUrl.append(JdbcUtils.AMPERSAND).append(config.get(JdbcUtils.JDBC_URL_PARAMS_KEY).asText());
     }

diff --git a/...ource-mysql/src/main/java/io/airbyte/integrations/source/mysql/MySqlSourceOperations.java b/...ource-mysql/src/main/java/io/airbyte/integrations/source/mysql/MySqlSourceOperations.java
@@ -43,7 +43,6 @@
 import com.mysql.cj.MysqlType;
 import com.mysql.cj.jdbc.result.ResultSetMetaData;
 import com.mysql.cj.result.Field;
-import io.airbyte.db.DataTypeUtils;
 import io.airbyte.db.SourceOperations;
 import io.airbyte.db.jdbc.AbstractJdbcCompatibleSourceOperations;
 import io.airbyte.protocol.models.JsonSchemaType;
@@ -95,7 +94,7 @@ public void copyToJsonField(final ResultSet resultSet, final int colIndex, final
           putShortInt(json, columnName, resultSet, colIndex);
         }
       }
-      case TINYINT_UNSIGNED -> putShortInt(json, columnName, resultSet, colIndex);
+      case TINYINT_UNSIGNED, YEAR -> putShortInt(json, columnName, resultSet, colIndex);
       case SMALLINT, SMALLINT_UNSIGNED, MEDIUMINT, MEDIUMINT_UNSIGNED -> putInteger(json, columnName, resultSet, colIndex);
       case INT, INT_UNSIGNED -> {
         if (field.isUnsigned()) {
@@ -112,18 +111,6 @@ public void copyToJsonField(final ResultSet resultSet, final int colIndex, final
       case DATETIME -> putTimestamp(json, columnName, resultSet, colIndex);
       case TIMESTAMP -> putTimestampWithTimezone(json, columnName, resultSet, colIndex);
       case TIME -> putTime(json, columnName, resultSet, colIndex);
-      // The returned year value can either be a java.sql.Short (when yearIsDateType=false)
-      // or a java.sql.Date with the date set to January 1st, at midnight (when yearIsDateType=true).
-      // Currently, JsonSchemaPrimitive does not support integer, but only supports number.
-      // Because the number type will be interpreted as a double in many destinations, and it is
-      // weird to show a year as a double, we set yearIsDateType=true in the JDBC connection string,
-      // and parse the returned year value as a string.
-      // The case can be re-evaluated when JsonSchemaPrimitive supports integer.
-      // Issue: https://github.com/airbytehq/airbyte/issues/8722
-      case YEAR -> {
-        final String year = resultSet.getDate(colIndex).toString().split("-")[0];
-        json.put(columnName, DataTypeUtils.returnNullIfInvalid(() -> year));
-      }
       case CHAR, VARCHAR -> {
         if (field.isBinary()) {
           // when character set is binary, the returned value is binary
@@ -156,7 +143,7 @@ public void setCursorField(final PreparedStatement preparedStatement,
     switch (cursorFieldType) {
       case BIT -> setBit(preparedStatement, parameterIndex, value);
       case BOOLEAN -> setBoolean(preparedStatement, parameterIndex, value);
-      case TINYINT, TINYINT_UNSIGNED, SMALLINT, SMALLINT_UNSIGNED, MEDIUMINT, MEDIUMINT_UNSIGNED -> setInteger(preparedStatement, parameterIndex,
+      case YEAR, TINYINT, TINYINT_UNSIGNED, SMALLINT, SMALLINT_UNSIGNED, MEDIUMINT, MEDIUMINT_UNSIGNED -> setInteger(preparedStatement, parameterIndex,
           value);
       case INT, INT_UNSIGNED, BIGINT, BIGINT_UNSIGNED -> setBigInteger(preparedStatement, parameterIndex, value);
       case FLOAT, FLOAT_UNSIGNED, DOUBLE, DOUBLE_UNSIGNED -> setDouble(preparedStatement, parameterIndex, value);
@@ -165,7 +152,7 @@ public void setCursorField(final PreparedStatement preparedStatement,
       case DATETIME -> setTimestamp(preparedStatement, parameterIndex, value);
       case TIMESTAMP -> setTimestampWithTimezone(preparedStatement, parameterIndex, value);
       case TIME -> setTime(preparedStatement, parameterIndex, value);
-      case YEAR, CHAR, VARCHAR, TINYTEXT, TEXT, MEDIUMTEXT, LONGTEXT, ENUM, SET -> setString(preparedStatement, parameterIndex, value);
+      case CHAR, VARCHAR, TINYTEXT, TEXT, MEDIUMTEXT, LONGTEXT, ENUM, SET -> setString(preparedStatement, parameterIndex, value);
       case TINYBLOB, BLOB, MEDIUMBLOB, LONGBLOB, BINARY, VARBINARY -> setBinary(preparedStatement, parameterIndex, value);
       // since cursor are expected to be comparable, handle cursor typing strictly and error on
       // unrecognized types
@@ -180,14 +167,16 @@ public MysqlType getDatabaseFieldType(final JsonNode field) {
       // e.g. MEDIUMINT UNSIGNED
       final MysqlType literalType = MysqlType.getByName(field.get(INTERNAL_COLUMN_TYPE_NAME).asText());
       final int columnSize = field.get(INTERNAL_COLUMN_SIZE).asInt();
-
       switch (literalType) {
         // BIT(1) and TINYINT(1) are interpreted as boolean
         case BIT, TINYINT -> {
           if (columnSize == 1) {
             return MysqlType.BOOLEAN;
           }
         }
+        case YEAR -> {
+          return SMALLINT;
+        }
         // When CHAR[N] and VARCHAR[N] columns have binary character set, the returned
         // types are BINARY[N] and VARBINARY[N], respectively. So we don't need to
         // convert them here. This is verified in MySqlSourceDatatypeTest.

diff --git a/...te/integrations/io/airbyte/integration_tests/sources/AbstractMySqlSourceDatatypeTest.java b/...te/integrations/io/airbyte/integration_tests/sources/AbstractMySqlSourceDatatypeTest.java
@@ -306,11 +306,11 @@ protected void initTests() {
     addDataTypeTestData(
         TestDataHolder.builder()
             .sourceType("year")
-            .airbyteType(JsonSchemaType.STRING)
+            .airbyteType(JsonSchemaType.INTEGER)
             // MySQL converts values in the ranges '0' - '69' to YEAR value in the range 2000 - 2069
             // and '70' - '99' to 1970 - 1999.
-            .addInsertValues("null", "'1997'", "'0'", "'50'", "'70'", "'80'", "'99'")
-            .addExpectedValues(null, "1997", "2000", "2050", "1970", "1980", "1999")
+            .addInsertValues("null", "'1997'", "'0'", "'50'", "'70'", "'80'", "'99'", "'00'", "'000'")
+            .addExpectedValues(null, "1997", "2000", "2050", "1970", "1980", "1999", "2000", "2000")
             .build());
 
     // char types can be string or binary, so they are tested separately

diff --git a/.../airbyte/integrations/io/airbyte/integration_tests/sources/MySqlDatatypeAccuracyTest.java b/.../airbyte/integrations/io/airbyte/integration_tests/sources/MySqlDatatypeAccuracyTest.java
@@ -326,13 +326,12 @@ protected void initTests() {
                   .build());
         }
         case YEAR -> {
-          /* https://github.com/airbytehq/airbyte/issues/8722 */
-//          addDataTypeTestData(
-//              TestDataHolder.builder()
-//                  .sourceType(mst.name())
-//                  .airbyteType(JsonSchemaType.INTEGER)
-//                  .fullSourceDataType("%s".formatted(mst.getName()))
-//                  .build());
+          addDataTypeTestData(
+              TestDataHolder.builder()
+                  .sourceType(mst.name())
+                  .airbyteType(JsonSchemaType.INTEGER)
+                  .fullSourceDataType("%s".formatted(mst.getName()))
+                  .build());
         }
         case VARCHAR -> {
           for (final Entry entry : charsetsCollationsMap.entrySet()) {