Skip to content

Commit 092ac4d

Browse files
authored
refactor: centralize type mapping to fix metadata-path divergence (#174)
## Why Column type information currently flows through three independent paths — Arrow schema, `pg_catalog.format_type()`, and Spark — each with its own per-site mapping logic. That scatter has caused repeated, silently-diverging bugs: `pg_catalog` rows returning raw PostgreSQL OIDs because `QueryMetadataUtil.dbTypeToSql` keyed on `int2`/`int4`/`int8` aliases that Hyper never emits; `ResultSetMetaData` disagreeing with `DatabaseMetaData.getColumns()` on type names and codes for the same column; DECIMAL vs NUMERIC inconsistency between the two paths; `getTypeInfo()` returning an empty result; engine-level concerns (parameter binding, vector population) reaching into JDBC exception types. ## Observable behavior changes **New capabilities / bug fixes:** - `DatabaseMetaData.getTypeInfo()` returns a real 18-column result enumerated from `HyperTypeKind` (was empty before). - Prepared-statement parameter Arrow round-trip: CHAR(1), CHAR(n), and VARCHAR(n) now round-trip through Arrow losslessly via `hyper:type` / `hyper:max_string_length` field metadata, matching the convention Hyper's server already uses on the inbound side. - `DatabaseMetaData.getColumns().COLUMN_SIZE` and `DECIMAL_DIGITS` are now derived from the parsed type's declared precision/scale (e.g. `numeric(10,5)` reports `COLUMN_SIZE=10, DECIMAL_DIGITS=5`; `char(1)` reports `COLUMN_SIZE=1`) instead of being hardcoded to `255` / `2`. - `DatabaseMetaData.getColumns().DATA_TYPE` is always a `java.sql.Types` value (was a raw PG OID for unmapped types); unsupported types surface as `Types.OTHER` with the raw name preserved via `HyperTypeKind.UNKNOWN` rather than throwing. - `DECIMAL_DIGITS` stays `0` for `FLOAT4` / `FLOAT8` (binary floats have no meaningful decimal scale) instead of being hardcoded to `2`. **Syntactic / naming changes:** - `DatabaseMetaData.getColumns().TYPE_NAME` is consistently JDBC-uppercase (`"BIGINT"`, `"INTEGER"`, `"DECIMAL"`, …) instead of a mix of Hyper-lowercase and uppercase. `INTERVAL` and `JSON` show the Hyper-uppercase form; unmodeled system-catalog types (e.g. `aclitem`) show the raw Hyper name. - Numeric/decimal unified to `DECIMAL` (both Arrow Decimal and Hyper `numeric` map to `HyperTypeKind.DECIMAL` / `JDBCType.DECIMAL`). ## What's introduced Introduce `HyperType`, a Hyper-native internal type model, and route every Arrow, `pg_catalog`, prepared-statement, and Spark conversion through it. The data layer lives under a new `com.salesforce.datacloud.jdbc.protocol.data` package that has no JDBC dependency, so engine-level callers can use it without a JDBC import; the JDBC-bound helpers stay in `com.salesforce.datacloud.jdbc.core.types.HyperTypes`. Translation to `java.sql.JDBCType` / `Types` only happens at the JDBC API boundary. **New in `protocol.data` (JDBC-free):** - `HyperTypeKind` + `HyperType` value class with a visitor and native `toHyperTypeName()`. `HyperTypeKind.UNKNOWN` preserves the raw Hyper type name for system-catalog types the driver doesn't model (e.g. `aclitem`). - `PgCatalogTypeParser`, `ArrowToHyperTypeMapper`, `HyperTypeToArrow` (format-boundary converters). - `ColumnMetadata` with optional `typeName` override. - `ParameterBinding` + `ParameterAccumulator` (replaces `ParameterManager`); throws `IllegalArgumentException` so it is usable without JDBC. - `ArrowUtils`, `VectorPopulator` (moved from `.util`). **`HyperTypes` stays in `core.types`** as the JDBC-bound boundary: `toJdbcType*`, `fromJdbcTypeCode`, `toJavaClass`, `getPrecision`/`getScale`/`getDisplaySize`, `isSigned`/`isCaseSensitive`/`isNullable`, `needsDecimalDigits`, `needsCharOctetLength`, `typeInfoRows`, plus the `isIntegerLike` / `isStringLike` helpers that wrap exhaustive enum switches so callers can do a family check without replicating the switch. ## Structural details **Deleted:** `ColumnType`, `ArrowToColumnTypeMapper`, the `QueryMetadataUtil.dbTypeToSql` + `isDecimalType`/`isCharType` helpers, the `ArrowUtils.SQL_TYPE_TO_FIELD_TYPE` map. **Migrated callers:** `ColumnMetadata` holds `HyperType`; `DataCloudResultSetMetaData`, `StreamingResultSet`, `DataCloudArray`, `DataCloudConnection.getSchemaForQueryId`, and `ArrowUtils.toColumnMetaData` go through `ArrowToHyperTypeMapper` + `HyperTypes`; `QueryMetadataUtil.constructColumnData` parses via `PgCatalogTypeParser` and falls back to `HyperType.unknown(raw)` for unmodeled system-catalog types; `SimpleResultSet` switches on `HyperTypeKind` via the new helpers (and picks up TINYINT numeric coercions and the `getFloat` wasNull/range-check fix that were planned as Phase 0 work); `DataCloudPreparedStatement` wraps `IllegalArgumentException` from `ParameterAccumulator` into `SQLException`; `VectorPopulator` is keyed by `HyperTypeKind`; Spark `TypeMapping` handles TINYINT, NUMERIC, CHAR, NULL, FLOAT, NCHAR, NVARCHAR, LONGNVARCHAR. **Follow-ups noted in code / comments (not in this PR):** - `SimpleResultSet` is documented for removal once `DataCloudMetadataResultSet` migrates to `StreamingResultSet` — one result-set implementation instead of two. - `QueryResultArrowStream.asArrowStreamReader`'s `RootAllocator` lifetime needs a dedicated fix (it flows into `ArrowStreamReader`, which doesn't close it). - `TIME WITH TIME ZONE` Arrow round-trip is pinned as intended asymmetry — offset honored on input but not preserved on output, matching Hyper's read-side behavior. Proper round-trip would need either a synthetic Arrow schema or Hyper-protocol coordination.
1 parent d4b1dcf commit 092ac4d

47 files changed

Lines changed: 3134 additions & 1577 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/ColumnNameResolver.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55
package com.salesforce.datacloud.jdbc.core;
66

7-
import com.salesforce.datacloud.jdbc.core.metadata.ColumnMetadata;
7+
import com.salesforce.datacloud.jdbc.protocol.data.ColumnMetadata;
88
import java.sql.SQLException;
99
import java.util.HashMap;
1010
import java.util.List;

jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/DataCloudConnection.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,9 @@
77
import static com.salesforce.datacloud.jdbc.config.DriverVersion.formatDriverInfo;
88
import static com.salesforce.datacloud.jdbc.exception.QueryExceptionHandler.createException;
99
import static com.salesforce.datacloud.jdbc.logging.ElapsedLogger.logTimedValue;
10-
import static com.salesforce.datacloud.jdbc.util.ArrowUtils.toColumnMetaData;
10+
import static com.salesforce.datacloud.jdbc.protocol.data.ArrowUtils.toColumnMetaData;
1111

1212
import com.google.protobuf.Empty;
13-
import com.salesforce.datacloud.jdbc.core.metadata.ColumnMetadata;
1413
import com.salesforce.datacloud.jdbc.core.metadata.DataCloudResultSetMetaData;
1514
import com.salesforce.datacloud.jdbc.core.partial.DataCloudQueryPolling;
1615
import com.salesforce.datacloud.jdbc.exception.DataCloudJDBCException;
@@ -22,6 +21,7 @@
2221
import com.salesforce.datacloud.jdbc.protocol.RowRangeIterator;
2322
import com.salesforce.datacloud.jdbc.protocol.async.core.AsyncStreamObserverIterator;
2423
import com.salesforce.datacloud.jdbc.protocol.async.core.SyncIteratorAdapter;
24+
import com.salesforce.datacloud.jdbc.protocol.data.ColumnMetadata;
2525
import com.salesforce.datacloud.jdbc.protocol.grpc.QueryAccessGrpcClient;
2626
import com.salesforce.datacloud.jdbc.util.Deadline;
2727
import com.salesforce.datacloud.jdbc.util.JdbcURL;
@@ -182,7 +182,7 @@ public PreparedStatement prepareStatement(String sql) {
182182
}
183183

184184
private DataCloudPreparedStatement getQueryPreparedStatement(String sql) {
185-
return new DataCloudPreparedStatement(this, sql, new DefaultParameterManager());
185+
return new DataCloudPreparedStatement(this, sql);
186186
}
187187

188188
/**
@@ -281,6 +281,12 @@ public ResultSetMetaData getSchemaForQueryId(String queryId) throws SQLException
281281
return new DataCloudResultSetMetaData(columns);
282282
} catch (StatusRuntimeException ex) {
283283
throw createException(connectionProperties.isIncludeCustomerDetailInReason(), null, queryId, ex);
284+
} catch (IllegalArgumentException ex) {
285+
// Thrown by ArrowToHyperTypeMapper for Arrow types the driver does not model.
286+
throw new SQLException(
287+
"Unsupported column type in query schema for queryId=" + queryId + ": " + ex.getMessage(),
288+
"0A000",
289+
ex);
284290
}
285291
}
286292

jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/DataCloudDatabaseMetadata.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
import com.google.common.collect.ImmutableList;
1515
import com.salesforce.datacloud.jdbc.config.DriverVersion;
16+
import com.salesforce.datacloud.jdbc.core.metadata.DataCloudResultSetMetaData;
17+
import com.salesforce.datacloud.jdbc.core.types.HyperTypes;
1618
import com.salesforce.datacloud.jdbc.util.JdbcURL;
1719
import com.salesforce.datacloud.jdbc.util.ThrowingJdbcSupplier;
1820
import java.sql.Connection;
@@ -753,7 +755,8 @@ public ResultSet getCrossReference(
753755

754756
@Override
755757
public ResultSet getTypeInfo() throws SQLException {
756-
return DataCloudMetadataResultSet.empty();
758+
return DataCloudMetadataResultSet.of(
759+
new DataCloudResultSetMetaData(MetadataSchemas.TYPE_INFO), HyperTypes.typeInfoRows());
757760
}
758761

759762
@Override

jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/DataCloudMetadataResultSet.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
*/
55
package com.salesforce.datacloud.jdbc.core;
66

7-
import com.salesforce.datacloud.jdbc.core.metadata.ColumnMetadata;
87
import com.salesforce.datacloud.jdbc.core.metadata.DataCloudResultSetMetaData;
98
import com.salesforce.datacloud.jdbc.core.resultset.ColumnAccessor;
109
import com.salesforce.datacloud.jdbc.core.resultset.SimpleResultSet;
10+
import com.salesforce.datacloud.jdbc.protocol.data.ColumnMetadata;
1111
import java.sql.SQLException;
1212
import java.sql.Statement;
1313
import java.util.Collections;

jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/DataCloudPreparedStatement.java

Lines changed: 53 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@
44
*/
55
package com.salesforce.datacloud.jdbc.core;
66

7-
import static com.salesforce.datacloud.jdbc.util.ArrowUtils.toArrowByteArray;
7+
import static com.salesforce.datacloud.jdbc.protocol.data.ArrowUtils.toArrowByteArray;
88
import static com.salesforce.datacloud.jdbc.util.DateTimeUtils.getUTCDateFromDateAndCalendar;
99
import static com.salesforce.datacloud.jdbc.util.DateTimeUtils.getUTCTimeFromTimeAndCalendar;
1010

1111
import com.google.common.collect.ImmutableMap;
1212
import com.google.common.collect.Maps;
1313
import com.google.protobuf.ByteString;
14+
import com.salesforce.datacloud.jdbc.protocol.data.HyperType;
15+
import com.salesforce.datacloud.jdbc.protocol.data.ParameterAccumulator;
1416
import com.salesforce.datacloud.jdbc.util.QueryTimeout;
1517
import com.salesforce.datacloud.jdbc.util.SqlErrorCodes;
1618
import java.io.IOException;
@@ -51,25 +53,28 @@
5153
@Slf4j
5254
public class DataCloudPreparedStatement extends DataCloudStatement implements PreparedStatement {
5355
private String sql;
54-
private final ParameterManager parameterManager;
56+
// Package-private for tests that need to inspect bound parameters.
57+
final ParameterAccumulator parameters = new ParameterAccumulator();
5558
private final Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
5659
// True if we are currently fetching metadata from the server, this influences the query param generation
5760
// to not return any data.
5861
private boolean fetchingMetadata = false;
5962

60-
DataCloudPreparedStatement(DataCloudConnection connection, ParameterManager parameterManager) {
63+
DataCloudPreparedStatement(DataCloudConnection connection) {
6164
super(connection);
62-
this.parameterManager = parameterManager;
6365
}
6466

65-
DataCloudPreparedStatement(DataCloudConnection connection, String sql, ParameterManager parameterManager) {
67+
DataCloudPreparedStatement(DataCloudConnection connection, String sql) {
6668
super(connection);
6769
this.sql = sql;
68-
this.parameterManager = parameterManager;
6970
}
7071

71-
private <T> void setParameter(int parameterIndex, int sqlType, T value) throws SQLException {
72-
parameterManager.setParameter(parameterIndex, sqlType, value);
72+
private <T> void setParameter(int parameterIndex, HyperType type, T value) throws SQLException {
73+
try {
74+
parameters.setParameter(parameterIndex, type, value);
75+
} catch (IllegalArgumentException ex) {
76+
throw new SQLException(ex.getMessage(), ex);
77+
}
7378
}
7479

7580
@Override
@@ -91,9 +96,13 @@ protected QueryParam.Builder getQueryParamBuilder(
9196

9297
final byte[] encodedRow;
9398
try {
94-
encodedRow = toArrowByteArray(parameterManager.getParameters(), calendar);
99+
encodedRow = toArrowByteArray(parameters.getParameters(), calendar);
95100
} catch (IOException e) {
96101
throw new SQLException("Failed to encode parameters on prepared statement", e);
102+
} catch (IllegalArgumentException e) {
103+
// Thrown when a parameter is bound with a HyperType we cannot currently encode as
104+
// Arrow (e.g. INTERVAL, JSON). Surface as a JDBC spec-compliant SQLException.
105+
throw new SQLException("Failed to encode parameters on prepared statement: " + e.getMessage(), "HY000", e);
97106
}
98107

99108
if (fetchingMetadata) {
@@ -131,52 +140,67 @@ public int executeUpdate() throws SQLException {
131140

132141
@Override
133142
public void setNull(int parameterIndex, int sqlType) throws SQLException {
134-
setParameter(parameterIndex, sqlType, null);
143+
setParameter(parameterIndex, hyperTypeForJdbcCode(sqlType), null);
144+
}
145+
146+
private static HyperType hyperTypeForJdbcCode(int sqlType) throws SQLException {
147+
try {
148+
return com.salesforce.datacloud.jdbc.core.types.HyperTypes.fromJdbcTypeCode(sqlType, true);
149+
} catch (IllegalArgumentException ex) {
150+
throw new SQLException("Unsupported JDBC type code: " + sqlType, "HYC00", ex);
151+
}
135152
}
136153

137154
@Override
138155
public void setBoolean(int parameterIndex, boolean x) throws SQLException {
139-
setParameter(parameterIndex, Types.BOOLEAN, x);
156+
setParameter(parameterIndex, HyperType.bool(false), x);
140157
}
141158

142159
@Override
143160
public void setByte(int parameterIndex, byte x) throws SQLException {
144-
setParameter(parameterIndex, Types.TINYINT, x);
161+
setParameter(parameterIndex, HyperType.int8(false), x);
145162
}
146163

147164
@Override
148165
public void setShort(int parameterIndex, short x) throws SQLException {
149-
setParameter(parameterIndex, Types.SMALLINT, x);
166+
setParameter(parameterIndex, HyperType.int16(false), x);
150167
}
151168

152169
@Override
153170
public void setInt(int parameterIndex, int x) throws SQLException {
154-
setParameter(parameterIndex, Types.INTEGER, x);
171+
setParameter(parameterIndex, HyperType.int32(false), x);
155172
}
156173

157174
@Override
158175
public void setLong(int parameterIndex, long x) throws SQLException {
159-
setParameter(parameterIndex, Types.BIGINT, x);
176+
setParameter(parameterIndex, HyperType.int64(false), x);
160177
}
161178

162179
@Override
163180
public void setFloat(int parameterIndex, float x) throws SQLException {
164-
setParameter(parameterIndex, Types.FLOAT, x);
181+
setParameter(parameterIndex, HyperType.float8(false), x);
165182
}
166183

167184
@Override
168185
public void setDouble(int parameterIndex, double x) throws SQLException {
169-
setParameter(parameterIndex, Types.DOUBLE, x);
186+
setParameter(parameterIndex, HyperType.float8(false), x);
170187
}
171188

172189
@Override
173190
public void setBigDecimal(int parameterIndex, BigDecimal x) throws SQLException {
174-
setParameter(parameterIndex, Types.DECIMAL, x);
191+
final HyperType type;
192+
if (x == null) {
193+
// Precision/scale are unknown; let Arrow encoding pick a suitable default.
194+
type = HyperType.decimal(0, 0, true);
195+
} else {
196+
type = HyperType.decimal(x.precision(), x.scale(), true);
197+
}
198+
setParameter(parameterIndex, type, x);
175199
}
176200

177201
@Override
178202
public void setString(int parameterIndex, String x) throws SQLException {
179-
setParameter(parameterIndex, Types.VARCHAR, x);
203+
setParameter(parameterIndex, HyperType.varcharUnlimited(true), x);
180204
}
181205

182206
@Override
@@ -186,17 +210,17 @@ public void setBytes(int parameterIndex, byte[] x) throws SQLException {
186210

187211
@Override
188212
public void setDate(int parameterIndex, Date x) throws SQLException {
189-
setParameter(parameterIndex, Types.DATE, x);
213+
setParameter(parameterIndex, HyperType.date(true), x);
190214
}
191215

192216
@Override
193217
public void setTime(int parameterIndex, Time x) throws SQLException {
194-
setParameter(parameterIndex, Types.TIME, x);
218+
setParameter(parameterIndex, HyperType.time(true), x);
195219
}
196220

197221
@Override
198222
public void setTimestamp(int parameterIndex, Timestamp x) throws SQLException {
199-
setParameter(parameterIndex, Types.TIMESTAMP, toWallClockAsUtc(x, null));
223+
setParameter(parameterIndex, HyperType.timestamp(true), toWallClockAsUtc(x, null));
200224
}
201225

202226
@Override
@@ -216,7 +240,7 @@ public void setBinaryStream(int parameterIndex, InputStream x, int length) throw
216240

217241
@Override
218242
public void clearParameters() {
219-
parameterManager.clearParameters();
243+
parameters.clearParameters();
220244
}
221245

222246
@Override
@@ -229,17 +253,17 @@ public void setObject(int parameterIndex, Object x, int targetSqlType) throws SQ
229253
// or store LocalDateTime digits directly.
230254
if (targetSqlType == Types.TIMESTAMP) {
231255
if (x instanceof Timestamp) {
232-
setParameter(parameterIndex, Types.TIMESTAMP, toWallClockAsUtc((Timestamp) x, null));
256+
setParameter(parameterIndex, HyperType.timestamp(true), toWallClockAsUtc((Timestamp) x, null));
233257
return;
234258
}
235259
if (x instanceof LocalDateTime) {
236260
LocalDateTime ldt = (LocalDateTime) x;
237261
// Encode LDT digits as if they were in UTC — no JVM-TZ shift applied.
238-
setParameter(parameterIndex, Types.TIMESTAMP, Timestamp.from(ldt.toInstant(ZoneOffset.UTC)));
262+
setParameter(parameterIndex, HyperType.timestamp(true), Timestamp.from(ldt.toInstant(ZoneOffset.UTC)));
239263
return;
240264
}
241265
}
242-
setParameter(parameterIndex, targetSqlType, x);
266+
setParameter(parameterIndex, hyperTypeForJdbcCode(targetSqlType), x);
243267
}
244268

245269
@Override
@@ -307,18 +331,18 @@ public ResultSetMetaData getMetaData() throws SQLException {
307331
@Override
308332
public void setDate(int parameterIndex, Date x, Calendar cal) throws SQLException {
309333
val utcDate = getUTCDateFromDateAndCalendar(x, cal);
310-
setParameter(parameterIndex, Types.DATE, utcDate);
334+
setParameter(parameterIndex, HyperType.date(true), utcDate);
311335
}
312336

313337
@Override
314338
public void setTime(int parameterIndex, Time x, Calendar cal) throws SQLException {
315339
val utcTime = getUTCTimeFromTimeAndCalendar(x, cal);
316-
setParameter(parameterIndex, Types.TIME, utcTime);
340+
setParameter(parameterIndex, HyperType.time(true), utcTime);
317341
}
318342

319343
@Override
320344
public void setTimestamp(int parameterIndex, Timestamp x, Calendar cal) throws SQLException {
321-
setParameter(parameterIndex, Types.TIMESTAMP, toWallClockAsUtc(x, cal));
345+
setParameter(parameterIndex, HyperType.timestamp(true), toWallClockAsUtc(x, cal));
322346
}
323347

324348
/**

0 commit comments

Comments
 (0)