From f33aff0120cec30164d2df389a4d77f08e829771 Mon Sep 17 00:00:00 2001 From: ohad Date: Thu, 6 Feb 2025 16:12:04 -0500 Subject: [PATCH 01/12] Initial implementation of prepared statement support in JDBC --- .../relational/api/SqlTypeNamesSupport.java | 28 ++++ .../jdbc/RelationalStructFacade.java | 2 +- .../relational/jdbc/TypeConversion.java | 82 +++++++++++ .../grpc/relational/jdbc/v1/column.proto | 18 +-- .../relational/jdbc/JDBCArrayImpl.java | 100 +++++++++++++ .../jdbc/JDBCRelationalConnection.java | 10 +- .../jdbc/JDBCRelationalPreparedStatement.java | 45 ++---- .../relational/jdbc/ParameterHelper.java | 137 ++++++++++++++++++ .../foundationdb/relational/server/FRL.java | 20 ++- .../parameterinjection/ListParameter.java | 2 +- .../test/java/JDBCYamlIntegrationTests.java | 7 - 11 files changed, 391 insertions(+), 60 deletions(-) create mode 100644 fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCArrayImpl.java create mode 100644 fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/ParameterHelper.java diff --git a/fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/SqlTypeNamesSupport.java b/fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/SqlTypeNamesSupport.java index c2afc3f991..34fa5be774 100644 --- a/fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/SqlTypeNamesSupport.java +++ b/fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/SqlTypeNamesSupport.java @@ -24,6 +24,8 @@ import com.apple.foundationdb.relational.util.ExcludeFromJacocoGeneratedReport; +import java.sql.Array; +import java.sql.Struct; import java.sql.Types; /** @@ -95,4 +97,30 @@ public static int getSqlTypeCode(String sqlTypeName) { throw new IllegalStateException("Unexpected sql type name:" + sqlTypeName); } } + + public static int getSqlTypeCodeFromObject(Object obj) { + if (obj == null) { + return Types.NULL; + } else if (obj instanceof Long) { + return Types.BIGINT; + } else if (obj instanceof Integer) { + return Types.INTEGER; + } else if (obj instanceof Boolean) { + return Types.BOOLEAN; + } else if (obj instanceof byte[]) { + return Types.BINARY; + } else if (obj instanceof Float) { + return Types.FLOAT; + } else if (obj instanceof Double) { + return Types.DOUBLE; + } else if (obj instanceof String) { + return Types.VARCHAR; + } else if (obj instanceof Array) { + return Types.ARRAY; + } else if (obj instanceof Struct) { + return Types.STRUCT; + } else { + throw new IllegalStateException("Unexpected object type: " + obj.getClass().getName()); + } + } } diff --git a/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/RelationalStructFacade.java b/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/RelationalStructFacade.java index 16683a5d95..11fe6116ab 100644 --- a/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/RelationalStructFacade.java +++ b/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/RelationalStructFacade.java @@ -311,7 +311,7 @@ public boolean wasNull() throws SQLException { private Column getColumnInternal(int oneBasedColumn) { Column c = this.delegate.getColumns().getColumn(PositionalIndex.toProtobuf(oneBasedColumn)); - wasNull = c.hasNull() || c.getKindCase().equals(Column.KindCase.KIND_NOT_SET); + wasNull = c.hasNullType() || c.getKindCase().equals(Column.KindCase.KIND_NOT_SET); return c; } diff --git a/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java b/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java index bef4cb0759..3279f1507f 100644 --- a/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java +++ b/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java @@ -241,6 +241,88 @@ private static Struct toStruct(RelationalStruct relationalStruct) throws SQLExce return Struct.newBuilder().setColumns(listColumnBuilder.build()).build(); } + /** + * Return the Java object stored within the proto. + * @param columnType the type of object in the column + * @param column the column to process + * @return the Java object from the Column representation + * @throws SQLException in case of an error + */ + public static Object fromColumn(int columnType, Column column) throws SQLException { + switch (columnType) { + case Types.ARRAY: + return fromArray(column.getArray()); + case Types.BIGINT: + return column.getLong(); + case Types.INTEGER: + return column.getInteger(); + case Types.BOOLEAN: + return column.getBoolean(); + case Types.VARCHAR: + return column.getString(); + case Types.BINARY: + return column.getBinary().toByteArray(); + case Types.DOUBLE: + return column.getDouble(); + default: + // TODO: NULL? + throw new SQLException("java.sql.Type=" + columnType + " not supported", + ErrorCode.UNSUPPORTED_OPERATION.getErrorCode()); + } + } + + /** + * Return the Java array stored within the proto. + * @param array the array to process + * @return the Java array from the proto representation + * @throws SQLException in case of an error + */ + public static Object[] fromArray(Array array) throws SQLException { + Object[] result = new Object[array.getElementCount()]; + final List elements = array.getElementList(); + for (int i = 0 ; i < elements.size() ; i++) { + result[i] = fromColumn(array.getElementType(), elements.get(i)); + } + return result; + } + + /** + * Create column from a Java object. + * @param columnType the SQL type to create + * @param obj the value to use for the column + * @return the created column + * @throws SQLException in case of error + */ + public static Column toColumn(int columnType, Object obj) throws SQLException { + Column.Builder builder = Column.newBuilder(); + switch (columnType) { + case Types.BIGINT: + builder = (obj == null) ? builder.clearLong() : builder.setLong((Long)obj); + break; + case Types.INTEGER: + builder = (obj == null) ? builder.clearInteger() : builder.setInteger((Integer)obj); + break; + case Types.BOOLEAN: + builder = (obj == null) ? builder.clearBoolean() : builder.setBoolean((Boolean)obj); + break; + case Types.VARCHAR: + builder = (obj == null) ? builder.clearString() : builder.setString((String)obj); + break; + case Types.BINARY: + builder = (obj == null) ? builder.clearBinary() : builder.setBinary((ByteString)obj); + break; + case Types.DOUBLE: + builder = (obj == null) ? builder.clearDouble() : builder.setDouble((Double)obj); + break; + default: + // TODO: NULL + // TODO Note that ARRAY type is not supported since we would need additional info (the element type) + throw new SQLException("java.sql.Type=" + columnType + " not supported", + ErrorCode.UNSUPPORTED_OPERATION.getErrorCode()); + } + return builder.build(); + } + private static Column toColumn(RelationalStruct relationalStruct, int oneBasedIndex) throws SQLException { int columnType = relationalStruct.getMetaData().getColumnType(oneBasedIndex); Column column; diff --git a/fdb-relational-grpc/src/main/proto/grpc/relational/jdbc/v1/column.proto b/fdb-relational-grpc/src/main/proto/grpc/relational/jdbc/v1/column.proto index 33054bc1f3..07f3dd0071 100644 --- a/fdb-relational-grpc/src/main/proto/grpc/relational/jdbc/v1/column.proto +++ b/fdb-relational-grpc/src/main/proto/grpc/relational/jdbc/v1/column.proto @@ -49,7 +49,10 @@ message Struct { // Relational Array. message Array { - repeated Column element = 1; + // The java.sql.Types of the elements of the array. This is somewhat redundant with the type of the + // columns, but it makes it easier to verify correctness. + int32 elementType = 1; + repeated Column element = 2; } // `Column` represents a dynamically typed column which can be either @@ -59,8 +62,8 @@ message Array { message Column { // The kind/type of column. oneof kind { - // Represents a null column. - NullColumn null = 1; + // Represents a null column. These can be typed, so the value is the java.sql.Types of the parameter + int32 nullType = 1; // Represents a double column. double double = 2; @@ -80,15 +83,6 @@ message Column { } } -// `NullValue` is a singleton enumeration to represent the null value for the -// `Column` type union. -// -// The JSON representation for `NullValue` is JSON `null`. -enum NullColumn { - // Null value. - NULL_COLUMN = 0; -} - // `ListColumn` is a wrapper around a repeated field of columns. message ListColumn { // Repeated field of dynamically typed columns. diff --git a/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCArrayImpl.java b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCArrayImpl.java new file mode 100644 index 0000000000..95178d0020 --- /dev/null +++ b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCArrayImpl.java @@ -0,0 +1,100 @@ +/* + * JDBCArrayImpl.java + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2015-2025 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.apple.foundationdb.relational.jdbc; + +import com.apple.foundationdb.relational.api.SqlTypeNamesSupport; + +import javax.annotation.Nonnull; +import java.sql.Array; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.SQLFeatureNotSupportedException; +import java.util.Map; + +/** + * A simplistic implementation of a {@link Array} that wraps around a + * {@link com.apple.foundationdb.relational.jdbc.grpc.v1.column.Array}. + * TODO: We can't use the other implementation of array in {@link RelationalArrayFacade} as it makes several assumptions + * that would be incompatible with the need to transfer an array across the wire (e.g. the type of element is missing, + * {@link RelationalArrayFacade#getArray()} returns a type other than a Java array). + */ +public class JDBCArrayImpl implements Array { + @Nonnull + private final com.apple.foundationdb.relational.jdbc.grpc.v1.column.Array underlying; + + public JDBCArrayImpl(@Nonnull final com.apple.foundationdb.relational.jdbc.grpc.v1.column.Array underlying) { + this.underlying = underlying; + } + + @Override + public String getBaseTypeName() throws SQLException { + return SqlTypeNamesSupport.getSqlTypeName(getBaseType()); + } + + @Override + public int getBaseType() throws SQLException { + return underlying.getElementType(); + } + + @Override + public Object getArray() throws SQLException { + return TypeConversion.fromArray(underlying); + } + + @Override + public Object getArray(final Map> map) throws SQLException { + throw new SQLFeatureNotSupportedException("Custom type mapping is not supported"); + } + + @Override + public Object getArray(final long index, final int count) throws SQLException { + throw new SQLFeatureNotSupportedException("Array slicing is not supported"); + } + + @Override + public Object getArray(final long index, final int count, final Map> map) throws SQLException { + throw new SQLFeatureNotSupportedException("Custom type mapping is not supported"); + } + + @Override + public ResultSet getResultSet() throws SQLException { + throw new SQLFeatureNotSupportedException("Array as result set is not supported"); + } + + @Override + public ResultSet getResultSet(final Map> map) throws SQLException { + throw new SQLFeatureNotSupportedException("Array as result set is not supported"); + } + + @Override + public ResultSet getResultSet(final long index, final int count) throws SQLException { + throw new SQLFeatureNotSupportedException("Array as result set is not supported"); + } + + @Override + public ResultSet getResultSet(final long index, final int count, final Map> map) throws SQLException { + throw new SQLFeatureNotSupportedException("Array as result set is not supported"); + } + + @Override + public void free() throws SQLException { + } +} diff --git a/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCRelationalConnection.java b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCRelationalConnection.java index 111e51683f..beebdb9803 100644 --- a/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCRelationalConnection.java +++ b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCRelationalConnection.java @@ -24,6 +24,7 @@ import com.apple.foundationdb.relational.api.RelationalConnection; import com.apple.foundationdb.relational.api.RelationalPreparedStatement; import com.apple.foundationdb.relational.api.RelationalStatement; +import com.apple.foundationdb.relational.api.SqlTypeNamesSupport; import com.apple.foundationdb.relational.api.exceptions.ErrorCode; import com.apple.foundationdb.relational.jdbc.grpc.GrpcConstants; import com.apple.foundationdb.relational.jdbc.grpc.v1.DatabaseMetaDataRequest; @@ -228,8 +229,13 @@ public void clearWarnings() throws SQLException { @Override public Array createArrayOf(String typeName, Object[] elements) throws SQLException { - // TODO: Implement - return null; + int elementType = SqlTypeNamesSupport.getSqlTypeCode(typeName); + final com.apple.foundationdb.relational.jdbc.grpc.v1.column.Array.Builder builder = com.apple.foundationdb.relational.jdbc.grpc.v1.column.Array.newBuilder(); + builder.setElementType(elementType); + for (Object element: elements) { + builder.addElement(TypeConversion.toColumn(elementType, element)); + } + return new JDBCArrayImpl(builder.build()); } @Override diff --git a/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCRelationalPreparedStatement.java b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCRelationalPreparedStatement.java index db92974d93..56bae31021 100644 --- a/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCRelationalPreparedStatement.java +++ b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCRelationalPreparedStatement.java @@ -24,17 +24,13 @@ import com.apple.foundationdb.relational.api.RelationalResultSet; import com.apple.foundationdb.relational.api.exceptions.ErrorCode; import com.apple.foundationdb.relational.jdbc.grpc.v1.Parameter; -import com.apple.foundationdb.relational.jdbc.grpc.v1.column.Column; import com.apple.foundationdb.relational.util.ExcludeFromJacocoGeneratedReport; -import com.google.protobuf.ByteString; - import javax.annotation.Nonnull; import java.sql.Array; import java.sql.Connection; import java.sql.SQLException; import java.sql.SQLWarning; -import java.sql.Types; import java.util.Map; import java.util.TreeMap; @@ -83,71 +79,52 @@ public int executeUpdate() throws SQLException { @Override public void setBoolean(int parameterIndex, boolean b) throws SQLException { - parameters.put(parameterIndex, - Parameter.newBuilder().setJavaSqlTypesCode(Types.BOOLEAN).setParameter(Column.newBuilder() - .setBoolean(b).build()).build()); + parameters.put(parameterIndex, ParameterHelper.ofBoolean(b)); } @Override public void setInt(int parameterIndex, int i) throws SQLException { - parameters.put(parameterIndex, - Parameter.newBuilder().setJavaSqlTypesCode(Types.INTEGER).setParameter(Column.newBuilder() - .setInteger(i).build()).build()); + parameters.put(parameterIndex, ParameterHelper.ofInt(i)); } @Override public void setLong(int parameterIndex, long l) throws SQLException { - parameters.put(parameterIndex, - Parameter.newBuilder().setJavaSqlTypesCode(Types.BIGINT).setParameter(Column.newBuilder() - .setLong(l).build()).build()); + parameters.put(parameterIndex, ParameterHelper.ofLong(l)); } @Override public void setFloat(int parameterIndex, float f) throws SQLException { - parameters.put(parameterIndex, - Parameter.newBuilder().setJavaSqlTypesCode(Types.FLOAT).setParameter(Column.newBuilder() - .setFloat(f).build()).build()); + parameters.put(parameterIndex, ParameterHelper.ofFloat(f)); } @Override public void setDouble(int parameterIndex, double d) throws SQLException { - parameters.put(parameterIndex, - Parameter.newBuilder().setJavaSqlTypesCode(Types.DOUBLE).setParameter(Column.newBuilder() - .setDouble(d).build()).build()); + parameters.put(parameterIndex, ParameterHelper.ofDouble(d)); } @Override public void setString(int parameterIndex, String s) throws SQLException { - parameters.put(parameterIndex, - Parameter.newBuilder().setJavaSqlTypesCode(Types.VARCHAR).setParameter(Column.newBuilder() - .setString(s).build()).build()); + parameters.put(parameterIndex, ParameterHelper.ofString(s)); } @Override public void setBytes(int parameterIndex, byte[] bytes) throws SQLException { - parameters.put(parameterIndex, - Parameter.newBuilder().setJavaSqlTypesCode(Types.BINARY).setParameter(Column.newBuilder() - .setBinary(ByteString.copyFrom(bytes)).build()).build()); + parameters.put(parameterIndex, ParameterHelper.ofBytes(bytes)); } @Override public void setObject(int parameterIndex, Object x) throws SQLException { - throw new SQLException("Not implemented in the relational layer " + - Thread.currentThread().getStackTrace()[1].getMethodName(), - ErrorCode.UNSUPPORTED_OPERATION.getErrorCode()); + parameters.put(parameterIndex, ParameterHelper.ofObject(x)); } @Override public void setNull(int parameterIndex, int sqlType) throws SQLException { - parameters.put(parameterIndex, - Parameter.newBuilder().setJavaSqlTypesCode(Types.NULL).build()); + parameters.put(parameterIndex, ParameterHelper.ofNull(sqlType)); } @Override - public void setArray(int parameterIndex, Array x) throws SQLException { - throw new SQLException("Not implemented in the relational layer " + - Thread.currentThread().getStackTrace()[1].getMethodName(), - ErrorCode.UNSUPPORTED_OPERATION.getErrorCode()); + public void setArray(int parameterIndex, Array a) throws SQLException { + parameters.put(parameterIndex, ParameterHelper.ofArray(a)); } @Override diff --git a/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/ParameterHelper.java b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/ParameterHelper.java new file mode 100644 index 0000000000..6534fb2ca1 --- /dev/null +++ b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/ParameterHelper.java @@ -0,0 +1,137 @@ +/* + * ParameterHelper.java + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2015-2025 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.apple.foundationdb.relational.jdbc; + +import com.apple.foundationdb.relational.api.SqlTypeNamesSupport; +import com.apple.foundationdb.relational.api.exceptions.ErrorCode; +import com.apple.foundationdb.relational.jdbc.grpc.v1.Parameter; +import com.apple.foundationdb.relational.jdbc.grpc.v1.column.Column; +import com.google.protobuf.ByteString; + +import java.sql.Array; +import java.sql.SQLException; +import java.sql.Types; +import java.util.ArrayList; +import java.util.List; + +public class ParameterHelper { + + public static Parameter ofBoolean(boolean b) { + return Parameter.newBuilder() + .setJavaSqlTypesCode(Types.BOOLEAN) + .setParameter(Column.newBuilder().setBoolean(b)) + .build(); + } + + public static Parameter ofInt(int i) { + return Parameter.newBuilder() + .setJavaSqlTypesCode(Types.INTEGER) + .setParameter(Column.newBuilder().setInteger(i)) + .build(); + } + + public static Parameter ofLong(long l) { + return Parameter.newBuilder() + .setJavaSqlTypesCode(Types.BIGINT) + .setParameter(Column.newBuilder().setLong(l)) + .build(); + } + + public static Parameter ofFloat(float f) { + return Parameter.newBuilder() + .setJavaSqlTypesCode(Types.FLOAT) + .setParameter(Column.newBuilder().setFloat(f)) + .build(); + } + + public static Parameter ofDouble(double d) { + return Parameter.newBuilder() + .setJavaSqlTypesCode(Types.DOUBLE) + .setParameter(Column.newBuilder().setDouble(d)) + .build(); + } + + public static Parameter ofString(String s) { + return Parameter.newBuilder() + .setJavaSqlTypesCode(Types.VARCHAR) + .setParameter(Column.newBuilder().setString(s)) + .build(); + } + + public static Parameter ofBytes(byte[] bytes) { + return Parameter.newBuilder() + .setJavaSqlTypesCode(Types.BINARY) + .setParameter(Column.newBuilder().setBinary(ByteString.copyFrom(bytes))) + .build(); + } + + public static Parameter ofNull(int sqlType) throws SQLException { + return Parameter.newBuilder() + .setJavaSqlTypesCode(Types.NULL) + .setParameter(Column.newBuilder().setNullType(sqlType)) + .build(); + } + + public static Parameter ofArray(final Array a) throws SQLException { + // TODO: Check type, we may be able to just copy columns + List elements = new ArrayList<>(); + Object[] arrayElements = (Object[])a.getArray(); + for (Object o: arrayElements) { + Parameter p = ofObject(o); + // TODO: Assert that the type matches the array type + elements.add(p.getParameter()); + } + return Parameter.newBuilder() + .setJavaSqlTypesCode(Types.ARRAY) + .setParameter(Column.newBuilder() + .setArray(com.apple.foundationdb.relational.jdbc.grpc.v1.column.Array.newBuilder() + .setElementType(a.getBaseType()) + .addAllElement(elements))) + .build(); + } + + public static Parameter ofObject(Object x) throws SQLException { + final int typeCodeFromObject = SqlTypeNamesSupport.getSqlTypeCodeFromObject(x); + switch (typeCodeFromObject) { + case Types.BIGINT: + return ofLong((Long)x); + case Types.INTEGER: + return ofInt((Integer)x); + case Types.BOOLEAN: + return ofBoolean((Boolean)x); + case Types.BINARY: + return ofBytes((byte[])x); + case Types.FLOAT: + return ofFloat((Float)x); + case Types.DOUBLE: + return ofDouble((Double)x); + case Types.VARCHAR: + return ofString((String)x); + case Types.NULL: + return ofNull(Types.NULL); // TODO: THis would be generic null... + case Types.ARRAY: + return ofArray((Array)x); + default: + throw new SQLException("setObject Not supported for type " + typeCodeFromObject, + ErrorCode.UNSUPPORTED_OPERATION.getErrorCode()); + } + } +} diff --git a/fdb-relational-server/src/main/java/com/apple/foundationdb/relational/server/FRL.java b/fdb-relational-server/src/main/java/com/apple/foundationdb/relational/server/FRL.java index 4867ac0016..c884b4b091 100644 --- a/fdb-relational-server/src/main/java/com/apple/foundationdb/relational/server/FRL.java +++ b/fdb-relational-server/src/main/java/com/apple/foundationdb/relational/server/FRL.java @@ -21,7 +21,6 @@ package com.apple.foundationdb.relational.server; import com.apple.foundationdb.annotation.API; - import com.apple.foundationdb.record.RecordCoreException; import com.apple.foundationdb.record.provider.foundationdb.FDBDatabase; import com.apple.foundationdb.record.provider.foundationdb.FDBDatabaseFactory; @@ -29,12 +28,13 @@ import com.apple.foundationdb.relational.api.EmbeddedRelationalDriver; import com.apple.foundationdb.relational.api.KeySet; import com.apple.foundationdb.relational.api.Options; -import com.apple.foundationdb.relational.api.Transaction; import com.apple.foundationdb.relational.api.RelationalDriver; import com.apple.foundationdb.relational.api.RelationalPreparedStatement; import com.apple.foundationdb.relational.api.RelationalResultSet; import com.apple.foundationdb.relational.api.RelationalStatement; import com.apple.foundationdb.relational.api.RelationalStruct; +import com.apple.foundationdb.relational.api.SqlTypeNamesSupport; +import com.apple.foundationdb.relational.api.Transaction; import com.apple.foundationdb.relational.api.catalog.StoreCatalog; import com.apple.foundationdb.relational.api.exceptions.RelationalException; import com.apple.foundationdb.relational.api.metrics.NoOpMetricRegistry; @@ -55,6 +55,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.net.URI; +import java.sql.Array; import java.sql.DriverManager; import java.sql.SQLException; import java.sql.Statement; @@ -230,11 +231,14 @@ private static void addPreparedStatementParameter(RelationalPreparedStatement re relationalPreparedStatement.setString(index, parameter.getParameter().getString()); break; case Types.BIGINT: - relationalPreparedStatement.setInt(index, parameter.getParameter().getInteger()); + relationalPreparedStatement.setLong(index, parameter.getParameter().getLong()); break; case Types.INTEGER: relationalPreparedStatement.setInt(index, parameter.getParameter().getInteger()); break; + case Types.FLOAT: + relationalPreparedStatement.setFloat(index, parameter.getParameter().getFloat()); + break; case Types.DOUBLE: relationalPreparedStatement.setDouble(index, parameter.getParameter().getDouble()); break; @@ -244,6 +248,16 @@ private static void addPreparedStatementParameter(RelationalPreparedStatement re case Types.BINARY: relationalPreparedStatement.setBytes(index, parameter.getParameter().getBinary().toByteArray()); break; + case Types.NULL: + relationalPreparedStatement.setNull(index, parameter.getParameter().getNullType()); + break; + case Types.ARRAY: + final com.apple.foundationdb.relational.jdbc.grpc.v1.column.Array arrayProto = parameter.getParameter().getArray(); + final Array relationalArray = relationalPreparedStatement.getConnection().createArrayOf( + SqlTypeNamesSupport.getSqlTypeName(arrayProto.getElementType()), + TypeConversion.fromArray(arrayProto)); + relationalPreparedStatement.setArray(index, relationalArray); + break; default: throw new SQLException("Unsupported type " + type); } diff --git a/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/command/parameterinjection/ListParameter.java b/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/command/parameterinjection/ListParameter.java index ef3f3b7125..4e0242adf1 100644 --- a/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/command/parameterinjection/ListParameter.java +++ b/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/command/parameterinjection/ListParameter.java @@ -76,7 +76,7 @@ public Object getSqlObject(Connection connection) throws SQLException { for (int i = 0; i < values.size(); i++) { array[i] = values.get(i).getSqlObject(connection); } - return Objects.requireNonNull(connection).createArrayOf(getSqlTypeName(array), array); + return Objects.requireNonNull(connection).createArrayOf(getSqlTypeName(array), array); // TODO: This should be the type of the element, not the array } // Best-effort approach to determine the type of constituent elements. diff --git a/yaml-tests/src/test/java/JDBCYamlIntegrationTests.java b/yaml-tests/src/test/java/JDBCYamlIntegrationTests.java index 1ce0731857..b210f96771 100644 --- a/yaml-tests/src/test/java/JDBCYamlIntegrationTests.java +++ b/yaml-tests/src/test/java/JDBCYamlIntegrationTests.java @@ -34,13 +34,6 @@ public void insertEnum() throws Exception { super.insertEnum(); } - @Override - @Test - @Disabled("TODO: Not supported") - public void prepared() throws Exception { - super.prepared(); - } - @Override @Test @Disabled("TODO: sql.Type OTHER is not supported in the ResultSet") From bc29a0362cdf1631c780e097fe77dbe928c63270 Mon Sep 17 00:00:00 2001 From: ohad Date: Thu, 6 Feb 2025 18:20:35 -0500 Subject: [PATCH 02/12] Add supported_version and support for nested arrays. --- .../relational/jdbc/TypeConversion.java | 29 ++++++++++++++++--- yaml-tests/src/test/resources/prepared.yamsql | 3 ++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java b/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java index 3279f1507f..b2d2ef71ec 100644 --- a/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java +++ b/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java @@ -287,8 +287,25 @@ public static Object[] fromArray(Array array) throws SQLException { } /** - * Create column from a Java object. - * @param columnType the SQL type to create + * Return the protobuf {@link Array} for a SQL {@link java.sql.Array}. + * @param array the SQL array + * @return the resulting protobuf array + */ + public static Array toArray(@Nonnull java.sql.Array array) throws SQLException { + Array.Builder builder = Array.newBuilder(); + builder.setElementType(array.getBaseType()); + for (Object o: (Object[])array.getArray()) { + builder.addElement(toColumn(array.getBaseType(), o)); + } + return builder.build(); + } + + /** + * Create {@link Column} from a Java object. + * Note: In case the column is of a composite type (array, struct) then the actual type has to be a SQL flavor + * ({@link java.sql.Array} or {@link java.sql.Struct}. + * Note: In case of {@link Types#NULL} null column, the obje parameter is expected to be the type of null + * @param columnType the SQL type to create (from {@link Types}) * @param obj the value to use for the column * @return the created column * @throws SQLException in case of error @@ -314,9 +331,13 @@ public static Column toColumn(int columnType, Object obj) throws SQLException { case Types.DOUBLE: builder = (obj == null) ? builder.clearDouble() : builder.setDouble((Double)obj); break; + case Types.ARRAY: + builder = (obj == null) ? builder.clearArray() : builder.setArray(toArray((java.sql.Array)obj)); + break; + case Types.NULL: + builder = builder.setNullType((Integer)obj); + break; default: - // TODO: NULL - // TODO Note that ARRAY type is not supported since we would need additional info (the element type) throw new SQLException("java.sql.Type=" + columnType + " not supported", ErrorCode.UNSUPPORTED_OPERATION.getErrorCode()); } diff --git a/yaml-tests/src/test/resources/prepared.yamsql b/yaml-tests/src/test/resources/prepared.yamsql index a0aede0170..8ef79aad14 100644 --- a/yaml-tests/src/test/resources/prepared.yamsql +++ b/yaml-tests/src/test/resources/prepared.yamsql @@ -17,6 +17,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +--- +options: + supported_version: !current_version --- schema_template: create table ta(a bigint, b double, c boolean, d integer, e integer array, f string, primary key(a)) From cec42ee4a9e69581d744f5866982eae2cc320c8b Mon Sep 17 00:00:00 2001 From: ohad Date: Thu, 6 Feb 2025 18:39:51 -0500 Subject: [PATCH 03/12] merge from Main, enable test for JDBC --- yaml-tests/src/test/java/YamlIntegrationTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/yaml-tests/src/test/java/YamlIntegrationTests.java b/yaml-tests/src/test/java/YamlIntegrationTests.java index 05a6b253c0..ec26d2bb70 100644 --- a/yaml-tests/src/test/java/YamlIntegrationTests.java +++ b/yaml-tests/src/test/java/YamlIntegrationTests.java @@ -255,7 +255,6 @@ public void insertEnum(YamlTest.Runner runner) throws Exception { } @TestTemplate - @ExcludeYamlTestConfig(value = YamlTestConfigExclusions.USES_JDBC, reason = "setObject is not supported by JDBC") public void prepared(YamlTest.Runner runner) throws Exception { runner.runYamsql("prepared.yamsql"); } From 9d6d6341630ca19e655ca8de867bf061b734a76e Mon Sep 17 00:00:00 2001 From: ohad Date: Thu, 6 Feb 2025 19:12:33 -0500 Subject: [PATCH 04/12] Optimize array creation to reuse the existing columns, protect against type mismatch in array. --- .../relational/jdbc/JDBCArrayImpl.java | 9 +++++++++ .../relational/jdbc/ParameterHelper.java | 20 +++++++++++++------ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCArrayImpl.java b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCArrayImpl.java index 95178d0020..408ac3a8ca 100644 --- a/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCArrayImpl.java +++ b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCArrayImpl.java @@ -97,4 +97,13 @@ public ResultSet getResultSet(final long index, final int count, final Map elements = new ArrayList<>(); - Object[] arrayElements = (Object[])a.getArray(); - for (Object o: arrayElements) { - Parameter p = ofObject(o); - // TODO: Assert that the type matches the array type - elements.add(p.getParameter()); + if (a instanceof JDBCArrayImpl) { + // we can shortcut the process and use the existing columns + JDBCArrayImpl arrayImpl = (JDBCArrayImpl)a; + elements.addAll(arrayImpl.getUnderlying().getElementList()); + } else { + // TODO: Do we even want to allow creation of parameter from an array created by another connection? + Object[] arrayElements = (Object[])a.getArray(); + for (Object o : arrayElements) { + Parameter p = ofObject(o); + if (p.getJavaSqlTypesCode() != a.getBaseType()) { + throw new SQLException("Array base type does not match element type: " + a.getBaseType() + ":" + p.getJavaSqlTypesCode()); + } + elements.add(p.getParameter()); + } } return Parameter.newBuilder() .setJavaSqlTypesCode(Types.ARRAY) From 2f2182ad7a1de8c38ce1bb7e3576283fafdcc264 Mon Sep 17 00:00:00 2001 From: ohad Date: Fri, 7 Feb 2025 08:56:20 -0500 Subject: [PATCH 05/12] Delete modified file --- yaml-tests/src/test/java/JDBCYamlIntegrationTests.java | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 yaml-tests/src/test/java/JDBCYamlIntegrationTests.java diff --git a/yaml-tests/src/test/java/JDBCYamlIntegrationTests.java b/yaml-tests/src/test/java/JDBCYamlIntegrationTests.java deleted file mode 100644 index e69de29bb2..0000000000 From f14fe3ec2b6249a42932e9354034840f9fead2c1 Mon Sep 17 00:00:00 2001 From: ohad Date: Fri, 7 Feb 2025 09:19:44 -0500 Subject: [PATCH 06/12] REmove Todo --- .../yamltests/command/parameterinjection/ListParameter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/command/parameterinjection/ListParameter.java b/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/command/parameterinjection/ListParameter.java index 4e0242adf1..ef3f3b7125 100644 --- a/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/command/parameterinjection/ListParameter.java +++ b/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/command/parameterinjection/ListParameter.java @@ -76,7 +76,7 @@ public Object getSqlObject(Connection connection) throws SQLException { for (int i = 0; i < values.size(); i++) { array[i] = values.get(i).getSqlObject(connection); } - return Objects.requireNonNull(connection).createArrayOf(getSqlTypeName(array), array); // TODO: This should be the type of the element, not the array + return Objects.requireNonNull(connection).createArrayOf(getSqlTypeName(array), array); } // Best-effort approach to determine the type of constituent elements. From 36e3ac697862b3ef6fac02ac16e5ba19b1c241f3 Mon Sep 17 00:00:00 2001 From: ohad Date: Fri, 7 Feb 2025 10:29:27 -0500 Subject: [PATCH 07/12] Fix test with nested arrays --- yaml-tests/src/test/resources/subquery-tests.yamsql | 1 + 1 file changed, 1 insertion(+) diff --git a/yaml-tests/src/test/resources/subquery-tests.yamsql b/yaml-tests/src/test/resources/subquery-tests.yamsql index e155e60869..732a033796 100644 --- a/yaml-tests/src/test/resources/subquery-tests.yamsql +++ b/yaml-tests/src/test/resources/subquery-tests.yamsql @@ -57,6 +57,7 @@ test_block: - # correlation are allowed in from-subquery. - query: select x, sq.idr, sq.nr from a, (select * from r where r.idr = a.x) sq; + - supported_version: !current_version # changes in Proto definition for arrays - result: [{x: 1, 1, [{11}, {12}, {13}]}, {x: 2, 2, [{21}, {22}, {23}]}, {x: 3, 3, [{31}, {32}, {33}]}] - # correlations are allowed inside a nested subquery with group by From e478fcf1231268e1a93205ca0f322a6a30f6f8e8 Mon Sep 17 00:00:00 2001 From: ohad Date: Fri, 7 Feb 2025 11:53:10 -0500 Subject: [PATCH 08/12] Exclude more tests with nested arrays --- yaml-tests/src/test/resources/arrays.yamsql | 3 +++ 1 file changed, 3 insertions(+) diff --git a/yaml-tests/src/test/resources/arrays.yamsql b/yaml-tests/src/test/resources/arrays.yamsql index aee86d1193..2132431227 100644 --- a/yaml-tests/src/test/resources/arrays.yamsql +++ b/yaml-tests/src/test/resources/arrays.yamsql @@ -37,6 +37,7 @@ test_block: - count: 3 - - query: SELECT * FROM A + - supported_version: !current_version # changes to array protobuf - result: [{1, [1, 2, 3]}, {2, [2, 3, 4]}, {3, [3, 4, 5]}] - - query: INSERT INTO B VALUES (1, ['a', 'b', 'c']), (2, ['b', 'c', 'd']), (3, ['c', 'd', 'e']) @@ -59,9 +60,11 @@ test_block: - # "new" should not be quoted. TODO ([Post] Fix identifiers case-sensitivity matching in plan generator) - query: UPDATE A SET X = [11, 12, 13] WHERE PK = 1 RETURNING "new".X + - supported_version: !current_version # changes to array protobuf - result: [{[11, 12, 13]}] - # "new" should not be quoted. TODO ([Post] Fix identifiers case-sensitivity matching in plan generator) - query: UPDATE D SET X = [(11), (12), (13)] WHERE PK = 1 RETURNING "new".X + - supported_version: !current_version # changes to array protobuf - result: [{[{F: 11}, {F: 12}, {F: 13}]}] ... From ea84095c12b9c4ca5bf11892ccfb40d3a4625f19 Mon Sep 17 00:00:00 2001 From: ohad Date: Tue, 11 Feb 2025 16:14:05 -0500 Subject: [PATCH 09/12] PR Comments --- .../relational/api/exceptions/ErrorCode.java | 1 + .../relational/jdbc/TypeConversion.java | 43 ++++++++++++++----- .../relational/jdbc/JDBCArrayImpl.java | 2 +- .../relational/jdbc/ParameterHelper.java | 2 +- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/exceptions/ErrorCode.java b/fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/exceptions/ErrorCode.java index 2278a0446e..a145bbc2a9 100644 --- a/fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/exceptions/ErrorCode.java +++ b/fdb-relational-api/src/main/java/com/apple/foundationdb/relational/api/exceptions/ErrorCode.java @@ -81,6 +81,7 @@ public enum ErrorCode { CANNOT_CONVERT_TYPE("22000"), INVALID_ROW_COUNT_IN_LIMIT_CLAUSE("2201W"), INVALID_PARAMETER("22023"), + ARRAY_ELEMENT_ERROR("2202E"), INVALID_BINARY_REPRESENTATION("22F03"), INVALID_ARGUMENT_FOR_FUNCTION("22F00"), diff --git a/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java b/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java index b2d2ef71ec..5cd5d43b48 100644 --- a/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java +++ b/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java @@ -24,6 +24,7 @@ import com.apple.foundationdb.relational.api.ArrayMetaData; import com.apple.foundationdb.relational.api.Continuation; +import com.apple.foundationdb.relational.api.SqlTypeNamesSupport; import com.apple.foundationdb.relational.api.StructMetaData; import com.apple.foundationdb.relational.api.RelationalArray; import com.apple.foundationdb.relational.api.RelationalResultSet; @@ -251,18 +252,25 @@ private static Struct toStruct(RelationalStruct relationalStruct) throws SQLExce public static Object fromColumn(int columnType, Column column) throws SQLException { switch (columnType) { case Types.ARRAY: + checkColumnType(columnType, column.hasArray()); return fromArray(column.getArray()); case Types.BIGINT: + checkColumnType(columnType, column.hasLong()); return column.getLong(); case Types.INTEGER: + checkColumnType(columnType, column.hasInteger()); return column.getInteger(); case Types.BOOLEAN: + checkColumnType(columnType, column.hasBoolean()); return column.getBoolean(); case Types.VARCHAR: + checkColumnType(columnType, column.hasString()); return column.getString(); case Types.BINARY: + checkColumnType(columnType, column.hasBinary()); return column.getBinary().toByteArray(); case Types.DOUBLE: + checkColumnType(columnType, column.hasDouble()); return column.getDouble(); default: // TODO: NULL? @@ -271,6 +279,12 @@ public static Object fromColumn(int columnType, Column column) throws SQLExcepti } } + private static void checkColumnType(final int expectedColumnType, final boolean columnHasType) throws SQLException { + if (!columnHasType) { + throw new SQLException("Column has wrong type (expected " + expectedColumnType + ")", ErrorCode.WRONG_OBJECT_TYPE.getErrorCode()); + } + } + /** * Return the Java array stored within the proto. * @param array the array to process @@ -302,37 +316,44 @@ public static Array toArray(@Nonnull java.sql.Array array) throws SQLException { /** * Create {@link Column} from a Java object. - * Note: In case the column is of a composite type (array, struct) then the actual type has to be a SQL flavor - * ({@link java.sql.Array} or {@link java.sql.Struct}. - * Note: In case of {@link Types#NULL} null column, the obje parameter is expected to be the type of null + * Note: In case the column is of a composite type (array) then the actual type has to be a SQL flavor + * ({@link java.sql.Array}. + * Note: In case {@code columnType} is of value {@link Types#NULL}, the {@code obj} parameter is expected to be the + * type of null. That is, the {@code obj} will represent the {@link Types} constant for the type of variable whose + * value is null. * @param columnType the SQL type to create (from {@link Types}) * @param obj the value to use for the column * @return the created column * @throws SQLException in case of error */ - public static Column toColumn(int columnType, Object obj) throws SQLException { + public static Column toColumn(int columnType, @Nonnull Object obj) throws SQLException { + if (columnType != SqlTypeNamesSupport.getSqlTypeCodeFromObject(obj)) { + throw new SQLException("Column element type does not match object type: " + columnType + " / " + obj.getClass().getSimpleName(), + ErrorCode.WRONG_OBJECT_TYPE.getErrorCode()); + } + Column.Builder builder = Column.newBuilder(); switch (columnType) { case Types.BIGINT: - builder = (obj == null) ? builder.clearLong() : builder.setLong((Long)obj); + builder = builder.setLong((Long)obj); break; case Types.INTEGER: - builder = (obj == null) ? builder.clearInteger() : builder.setInteger((Integer)obj); + builder = builder.setInteger((Integer)obj); break; case Types.BOOLEAN: - builder = (obj == null) ? builder.clearBoolean() : builder.setBoolean((Boolean)obj); + builder = builder.setBoolean((Boolean)obj); break; case Types.VARCHAR: - builder = (obj == null) ? builder.clearString() : builder.setString((String)obj); + builder = builder.setString((String)obj); break; case Types.BINARY: - builder = (obj == null) ? builder.clearBinary() : builder.setBinary((ByteString)obj); + builder = builder.setBinary((ByteString)obj); break; case Types.DOUBLE: - builder = (obj == null) ? builder.clearDouble() : builder.setDouble((Double)obj); + builder = builder.setDouble((Double)obj); break; case Types.ARRAY: - builder = (obj == null) ? builder.clearArray() : builder.setArray(toArray((java.sql.Array)obj)); + builder = builder.setArray(toArray((java.sql.Array)obj)); break; case Types.NULL: builder = builder.setNullType((Integer)obj); diff --git a/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCArrayImpl.java b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCArrayImpl.java index 408ac3a8ca..3edb9d0505 100644 --- a/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCArrayImpl.java +++ b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/JDBCArrayImpl.java @@ -103,7 +103,7 @@ public void free() throws SQLException { * @return the underlying protobuf struct */ @Nonnull - public com.apple.foundationdb.relational.jdbc.grpc.v1.column.Array getUnderlying() { + com.apple.foundationdb.relational.jdbc.grpc.v1.column.Array getUnderlying() { return underlying; } } diff --git a/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/ParameterHelper.java b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/ParameterHelper.java index 6eb05a6272..b6becf17ef 100644 --- a/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/ParameterHelper.java +++ b/fdb-relational-jdbc/src/main/java/com/apple/foundationdb/relational/jdbc/ParameterHelper.java @@ -102,7 +102,7 @@ public static Parameter ofArray(final Array a) throws SQLException { for (Object o : arrayElements) { Parameter p = ofObject(o); if (p.getJavaSqlTypesCode() != a.getBaseType()) { - throw new SQLException("Array base type does not match element type: " + a.getBaseType() + ":" + p.getJavaSqlTypesCode()); + throw new SQLException("Array base type does not match element type: " + a.getBaseType() + ":" + p.getJavaSqlTypesCode(), ErrorCode.ARRAY_ELEMENT_ERROR.getErrorCode()); } elements.add(p.getParameter()); } From 3cacdfad73df46cb896bb34b7363c767a04178a3 Mon Sep 17 00:00:00 2001 From: ohad Date: Tue, 11 Feb 2025 17:46:35 -0500 Subject: [PATCH 10/12] PR Comments: Make protobuf changes backwards compatible, add checks where needed for old server support --- .../relational/jdbc/TypeConversion.java | 6 +++--- .../proto/grpc/relational/jdbc/v1/column.proto | 17 ++++++++++++----- yaml-tests/src/test/resources/arrays.yamsql | 3 --- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java b/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java index 5cd5d43b48..10b1b1f6b1 100644 --- a/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java +++ b/fdb-relational-grpc/src/main/java/com/apple/foundationdb/relational/jdbc/TypeConversion.java @@ -273,9 +273,9 @@ public static Object fromColumn(int columnType, Column column) throws SQLExcepti checkColumnType(columnType, column.hasDouble()); return column.getDouble(); default: - // TODO: NULL? - throw new SQLException("java.sql.Type=" + columnType + " not supported", - ErrorCode.UNSUPPORTED_OPERATION.getErrorCode()); + // NULL (java.sql.Types value 0) is not a valid column type for an array and is likely the result of a default value for the + // (optional) array.getElementType() protobuf field. + throw new SQLException("java.sql.Type=" + columnType + " not supported", ErrorCode.ARRAY_ELEMENT_ERROR.getErrorCode()); } } diff --git a/fdb-relational-grpc/src/main/proto/grpc/relational/jdbc/v1/column.proto b/fdb-relational-grpc/src/main/proto/grpc/relational/jdbc/v1/column.proto index 07f3dd0071..d79f44bc52 100644 --- a/fdb-relational-grpc/src/main/proto/grpc/relational/jdbc/v1/column.proto +++ b/fdb-relational-grpc/src/main/proto/grpc/relational/jdbc/v1/column.proto @@ -49,10 +49,10 @@ message Struct { // Relational Array. message Array { + repeated Column element = 1; // The java.sql.Types of the elements of the array. This is somewhat redundant with the type of the // columns, but it makes it easier to verify correctness. - int32 elementType = 1; - repeated Column element = 2; + int32 elementType = 2; } // `Column` represents a dynamically typed column which can be either @@ -62,8 +62,8 @@ message Array { message Column { // The kind/type of column. oneof kind { - // Represents a null column. These can be typed, so the value is the java.sql.Types of the parameter - int32 nullType = 1; + // Deprecated + NullColumn null = 1; // Represents a double column. double double = 2; @@ -78,11 +78,18 @@ message Column { Struct struct = 7; Array array = 8; bytes binary = 9; - float float = 10; + // Represents a null value. These can be typed, so the value is the java.sql.Types of the parameter + int32 nullType = 11; } } +// Deprecated +enum NullColumn { + // Null value. + NULL_COLUMN = 0; +} + // `ListColumn` is a wrapper around a repeated field of columns. message ListColumn { // Repeated field of dynamically typed columns. diff --git a/yaml-tests/src/test/resources/arrays.yamsql b/yaml-tests/src/test/resources/arrays.yamsql index 693428d5c4..2f0be34df9 100644 --- a/yaml-tests/src/test/resources/arrays.yamsql +++ b/yaml-tests/src/test/resources/arrays.yamsql @@ -38,7 +38,6 @@ test_block: - count: 3 - - query: SELECT * FROM A - - supported_version: !current_version # changes to array protobuf - result: [{1, [1, 2, 3]}, {2, [2, 3, 4]}, {3, [3, 4, 5]}] - - query: INSERT INTO B VALUES (1, ['a', 'b', 'c']), (2, ['b', 'c', 'd']), (3, ['c', 'd', 'e']) @@ -61,11 +60,9 @@ test_block: - # "new" should not be quoted. TODO ([Post] Fix identifiers case-sensitivity matching in plan generator) - query: UPDATE A SET X = [11, 12, 13] WHERE PK = 1 RETURNING "new".X - - supported_version: !current_version # changes to array protobuf - result: [{[11, 12, 13]}] - # "new" should not be quoted. TODO ([Post] Fix identifiers case-sensitivity matching in plan generator) - query: UPDATE D SET X = [(11), (12), (13)] WHERE PK = 1 RETURNING "new".X - - supported_version: !current_version # changes to array protobuf - result: [{[{F: 11}, {F: 12}, {F: 13}]}] ... From 231affc12ecf774be474a649f7ccaa9471cd843f Mon Sep 17 00:00:00 2001 From: ohad Date: Tue, 11 Feb 2025 17:50:58 -0500 Subject: [PATCH 11/12] Remove exclusion on prepared.yamsql --- yaml-tests/src/test/java/YamlIntegrationTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/yaml-tests/src/test/java/YamlIntegrationTests.java b/yaml-tests/src/test/java/YamlIntegrationTests.java index e8757de0e7..999a532ff1 100644 --- a/yaml-tests/src/test/java/YamlIntegrationTests.java +++ b/yaml-tests/src/test/java/YamlIntegrationTests.java @@ -259,7 +259,6 @@ public void insertEnum(YamlTest.Runner runner) throws Exception { } @TestTemplate - @ExcludeYamlTestConfig(value = YamlTestConfigFilters.DO_NOT_USE_JDBC, reason = "setObject is not supported by JDBC") public void prepared(YamlTest.Runner runner) throws Exception { runner.runYamsql("prepared.yamsql"); } From dd322182762de6fddc3e0b3b8651c2c52bbfb09b Mon Sep 17 00:00:00 2001 From: ohad Date: Thu, 13 Feb 2025 12:53:23 -0500 Subject: [PATCH 12/12] PR comments --- yaml-tests/src/test/resources/subquery-tests.yamsql | 1 - 1 file changed, 1 deletion(-) diff --git a/yaml-tests/src/test/resources/subquery-tests.yamsql b/yaml-tests/src/test/resources/subquery-tests.yamsql index 2a2e3dc66f..03c8dbdd4f 100644 --- a/yaml-tests/src/test/resources/subquery-tests.yamsql +++ b/yaml-tests/src/test/resources/subquery-tests.yamsql @@ -58,7 +58,6 @@ test_block: - # correlation are allowed in from-subquery. - query: select x, sq.idr, sq.nr from a, (select * from r where r.idr = a.x) sq; - - supported_version: !current_version # changes in Proto definition for arrays - result: [{x: 1, 1, [{11}, {12}, {13}]}, {x: 2, 2, [{21}, {22}, {23}]}, {x: 3, 3, [{31}, {32}, {33}]}] - # correlations are allowed inside a nested subquery with group by