diff --git a/doc/changes/changes_18.0.2.md b/doc/changes/changes_18.0.2.md index b0dbc516..2ddd28fa 100644 --- a/doc/changes/changes_18.0.2.md +++ b/doc/changes/changes_18.0.2.md @@ -7,11 +7,13 @@ Code name: Improve code quality ## Breaking Changes * `Capabilities.subtractCapabilities()` is now deprecated for removal and delegates to the new pure `Capabilities.subtract()` implementation. Code that relied on the previous side effect of mutating the receiver must be adapted (#305). +* Constructor `ColumnMetadata.Builder()` is now deprecated for removal. Use `ColumnMetadata.builder()` to create a new instance (#306). * Constructor `Capabilities.Builder()` is now deprecated for removal. Use `Capabilities.builder()` to create a new instance (#305). ## Bugfixes * #305: Fixed `Capabilities.subtractCapabilities()` so it no longer mutates the receiver. Added the pure `Capabilities.subtract()` method. +* #306: Fixed `TablesMetadataParser` so omitted `isIdentity` values default to `false` while omitted `isNullable` values still default to `true`. ## Dependency Updates diff --git a/src/main/java/com/exasol/adapter/metadata/ColumnMetadata.java b/src/main/java/com/exasol/adapter/metadata/ColumnMetadata.java index 93ef4046..1aec042d 100644 --- a/src/main/java/com/exasol/adapter/metadata/ColumnMetadata.java +++ b/src/main/java/com/exasol/adapter/metadata/ColumnMetadata.java @@ -192,6 +192,14 @@ public static class Builder { private String originalTypeName = null; private String comment = ""; + /** + * @deprecated use {@link ColumnMetadata#builder()} instead. + */ + @Deprecated(since = "18.0.2", forRemoval = true) + public Builder() { + // intentionally left blank + } + /** * Set the column name * diff --git a/src/main/java/com/exasol/adapter/request/parser/TablesMetadataParser.java b/src/main/java/com/exasol/adapter/request/parser/TablesMetadataParser.java index f6977522..b401a1eb 100644 --- a/src/main/java/com/exasol/adapter/request/parser/TablesMetadataParser.java +++ b/src/main/java/com/exasol/adapter/request/parser/TablesMetadataParser.java @@ -57,8 +57,8 @@ private ColumnMetadata parseColumnMetadata(final JsonObject column) { final String adapterNotes = readAdapterNotes(column); final String comment = column.getString(TABLE_COMMENT_KEY, ""); final String defaultValue = column.getString("default", ""); - final boolean isNullable = applyBooleanValue(column, "isNullable"); - final boolean isIdentity = applyBooleanValue(column, "isIdentity"); + final boolean isNullable = applyBooleanValue(column, "isNullable", true); + final boolean isIdentity = applyBooleanValue(column, "isIdentity", false); final JsonObject dataType = column.getJsonObject(DATA_TYPE); final DataType type = getDataType(dataType); return ColumnMetadata.builder().name(columnName).adapterNotes(adapterNotes).type(type).nullable(isNullable) @@ -81,11 +81,12 @@ private String getAdapterNotesString(final JsonValue notes) { } } - private boolean applyBooleanValue(final JsonObject column, final String bolleanName) { - if (column.containsKey(bolleanName)) { - return column.getBoolean(bolleanName); + private boolean applyBooleanValue(final JsonObject column, final String booleanName, + final boolean defaultValue) { + if (column.containsKey(booleanName)) { + return column.getBoolean(booleanName); } - return true; + return defaultValue; } private static DataType.ExaCharset charSetFromString(final String charset) { diff --git a/src/test/java/com/exasol/adapter/metadata/converter/SchemaMetadataJsonConverterTest.java b/src/test/java/com/exasol/adapter/metadata/converter/SchemaMetadataJsonConverterTest.java index c250b669..fc7d7f88 100644 --- a/src/test/java/com/exasol/adapter/metadata/converter/SchemaMetadataJsonConverterTest.java +++ b/src/test/java/com/exasol/adapter/metadata/converter/SchemaMetadataJsonConverterTest.java @@ -59,12 +59,12 @@ void testConvert() throws JSONException { + "}"; final List tables = new ArrayList<>(); final List columnsA = new ArrayList<>(); - columnsA.add(new ColumnMetadata.Builder().name("column_A1").comment("comment A1").type(DataType.createDouble()) + columnsA.add(ColumnMetadata.builder().name("column_A1").comment("comment A1").type(DataType.createDouble()) .adapterNotes("notes A1").nullable(false).build()); - columnsA.add(new ColumnMetadata.Builder().name("column_A2").type(DataType.createDate()) + columnsA.add(ColumnMetadata.builder().name("column_A2").type(DataType.createDate()) .defaultValue("default A2").identity(true).build()); final List columnsB = new ArrayList<>(); - columnsB.add(new ColumnMetadata.Builder().name("COLUMN_B1").type(DataType.createBool()).build()); + columnsB.add(ColumnMetadata.builder().name("COLUMN_B1").type(DataType.createBool()).build()); tables.add(new TableMetadata("table_A", "notes A", columnsA, "comment A")); tables.add(new TableMetadata("TABLE_B", null, columnsB, null)); final SchemaMetadata schemaMetadata = new SchemaMetadata(SCHEMA_NAME, tables); diff --git a/src/test/java/com/exasol/adapter/request/parser/TablesMetadataParserTest.java b/src/test/java/com/exasol/adapter/request/parser/TablesMetadataParserTest.java index defabb28..983bc023 100644 --- a/src/test/java/com/exasol/adapter/request/parser/TablesMetadataParserTest.java +++ b/src/test/java/com/exasol/adapter/request/parser/TablesMetadataParserTest.java @@ -3,6 +3,7 @@ import static com.exasol.adapter.metadata.DataType.ExaCharset.ASCII; import static com.exasol.adapter.metadata.DataType.ExaCharset.UTF8; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; import java.io.*; @@ -22,14 +23,14 @@ class TablesMetadataParserTest { void testParseMetadata() throws IOException { final List tableColumns = new ArrayList<>(); tableColumns.add(ColumnMetadata.builder().name("ID").adapterNotes("").type(DataType.createDecimal(22, 0)) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add(ColumnMetadata.builder().name("USER_ID").adapterNotes("").type(DataType.createDecimal(18, 0)) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add(ColumnMetadata.builder().name("URL").adapterNotes("").type(DataType.createVarChar(1000, UTF8)) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add( ColumnMetadata.builder().name("REQUEST_TIME").adapterNotes("").type(DataType.createTimestamp(false, 3)) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); final List expectedInvolvedTablesMetadata = new ArrayList<>(); expectedInvolvedTablesMetadata.add(new TableMetadata("CLICKS", "", tableColumns, "")); final JsonArray tablesAsJson = readInvolvedTablesFromJsonFile("target/test-classes/pushdown_request.json"); @@ -60,59 +61,90 @@ void testParseTablesMetadataAllColumnsTypes() throws IOException { private List createExpectedTableMetadata() { final List tableColumns = new ArrayList<>(); tableColumns.add(ColumnMetadata.builder().name("C_DECIMAL").adapterNotes("").type(DataType.createDecimal(18, 2)) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add(ColumnMetadata.builder().name("C_DOUBLE").adapterNotes("").type(DataType.createDouble()) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add(ColumnMetadata.builder().name("C_VARCHAR_UTF8_1").adapterNotes("") - .type(DataType.createVarChar(10000, UTF8)).nullable(true).identity(true).defaultValue("").comment("") + .type(DataType.createVarChar(10000, UTF8)).nullable(true).identity(false).defaultValue("").comment("") .build()); tableColumns.add(ColumnMetadata.builder().name("C_VARCHAR_UTF8_2").adapterNotes("") - .type(DataType.createVarChar(10000, UTF8)).nullable(true).identity(true).defaultValue("").comment("") + .type(DataType.createVarChar(10000, UTF8)).nullable(true).identity(false).defaultValue("").comment("") .build()); tableColumns.add(ColumnMetadata.builder().name("C_VARCHAR_ASCII").adapterNotes("") - .type(DataType.createVarChar(10000, ASCII)).nullable(true).identity(true).defaultValue("").comment("") + .type(DataType.createVarChar(10000, ASCII)).nullable(true).identity(false).defaultValue("").comment("") .build()); tableColumns.add(ColumnMetadata.builder().name("C_CHAR_UTF8_1").adapterNotes("") - .type(DataType.createChar(3, UTF8)).nullable(true).identity(true).defaultValue("").comment("").build()); + .type(DataType.createChar(3, UTF8)).nullable(true).identity(false).defaultValue("").comment("") + .build()); tableColumns.add(ColumnMetadata.builder().name("C_CHAR_UTF8_2").adapterNotes("") - .type(DataType.createChar(3, UTF8)).nullable(true).identity(true).defaultValue("").comment("").build()); + .type(DataType.createChar(3, UTF8)).nullable(true).identity(false).defaultValue("").comment("") + .build()); tableColumns .add(ColumnMetadata.builder().name("C_CHAR_ASCII").adapterNotes("").type(DataType.createChar(3, ASCII)) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add(ColumnMetadata.builder().name("C_DATE").adapterNotes("").type(DataType.createDate()) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add( ColumnMetadata.builder().name("C_TIMESTAMP_1").adapterNotes("").type(DataType.createTimestamp(false, 3)) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add( ColumnMetadata.builder().name("C_TIMESTAMP_2").adapterNotes("").type(DataType.createTimestamp(false, 3)) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add( ColumnMetadata.builder().name("C_TIMESTAMP_3").adapterNotes("").type(DataType.createTimestamp(true, 3)) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add( ColumnMetadata.builder().name("C_TIMESTAMP_4").adapterNotes("").type(DataType.createTimestamp(false, 7)) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add(ColumnMetadata.builder().name("C_BOOLEAN").adapterNotes("").type(DataType.createBool()) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add(ColumnMetadata.builder().name("C_GEOMETRY").adapterNotes("").type(DataType.createGeometry(1)) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add(ColumnMetadata.builder().name("C_HASHTYPE").adapterNotes("").type(DataType.createHashtype(16)) - .nullable(true).identity(true).defaultValue("").comment("").build()); + .nullable(true).identity(false).defaultValue("").comment("").build()); tableColumns.add(ColumnMetadata.builder().name("C_INTERVAL_DS_1").adapterNotes("") - .type(DataType.createIntervalDaySecond(2, 3)).nullable(true).identity(true).defaultValue("").comment("") + .type(DataType.createIntervalDaySecond(2, 3)).nullable(true).identity(false).defaultValue("") + .comment("") .build()); tableColumns.add(ColumnMetadata.builder().name("C_INTERVAL_DS_2").adapterNotes("") - .type(DataType.createIntervalDaySecond(3, 4)).nullable(true).identity(true).defaultValue("").comment("") + .type(DataType.createIntervalDaySecond(3, 4)).nullable(true).identity(false).defaultValue("") + .comment("") .build()); tableColumns.add(ColumnMetadata.builder().name("C_INTERVAL_YM_1").adapterNotes("") - .type(DataType.createIntervalYearMonth(2)).nullable(true).identity(true).defaultValue("").comment("") + .type(DataType.createIntervalYearMonth(2)).nullable(true).identity(false).defaultValue("").comment("") .build()); tableColumns.add(ColumnMetadata.builder().name("C_INTERVAL_YM_2").adapterNotes("") - .type(DataType.createIntervalYearMonth(3)).nullable(true).identity(true).defaultValue("").comment("") + .type(DataType.createIntervalYearMonth(3)).nullable(true).identity(false).defaultValue("").comment("") .build()); final List expectedInvolvedTablesMetadata = new ArrayList<>(); expectedInvolvedTablesMetadata.add(new TableMetadata("T1", "", tableColumns, "")); return expectedInvolvedTablesMetadata; } + + @Test + void testParseColumnMetadataUsesBooleanDefaults() { + final JsonArray tablesAsJson = Json.createArrayBuilder().add(Json.createObjectBuilder() + .add("name", "T1") + .add("columns", Json.createArrayBuilder() + .add(Json.createObjectBuilder().add("name", "DEFAULTS") + .add("dataType", Json.createObjectBuilder().add("type", "DECIMAL").add("precision", 18) + .add("scale", 0))) + .add(Json.createObjectBuilder().add("name", "EXPLICIT_VALUES").add("isNullable", false) + .add("isIdentity", true) + .add("dataType", Json.createObjectBuilder().add("type", "DECIMAL").add("precision", 18) + .add("scale", 0))))) + .build(); + + final List tables = TablesMetadataParser.create().parse(tablesAsJson); + + final List expectedColumns = List.of( + ColumnMetadata.builder().name("DEFAULTS").adapterNotes("") + .type(DataType.createDecimal(18, 0)).nullable(true).identity(false).defaultValue("").comment("") + .build(), + ColumnMetadata.builder().name("EXPLICIT_VALUES").adapterNotes("") + .type(DataType.createDecimal(18, 0)).nullable(false).identity(true).defaultValue("").comment("") + .build()); + + assertThat(tables, contains(new TableMetadata("T1", "", expectedColumns, ""))); + } }