Skip to content

Commit 6817dde

Browse files
committed
fix: provide default getObject(Class) fallback in QueryJDBCAccessor
QueryJDBCAccessor.getObject(Class) threw "Operation not supported" no matter what. That's a problem: JDBC says ResultSet.getObject(int, Class<T>) is supposed to return the value as T when the conversion is trivial, but every accessor that didn't override it would throw, even on the identity case like getObject(col, String.class) against a VARCHAR. Callers had to either know which accessors implement typed conversion or wrap calls in catch-and-retry. The new default covers the JDBC 4.2 conversions that every accessor already exposes through its primitive getters, in this resolution order: 1. null type -> SQLException with SQLState 22023. 2. String.class -> delegate to getString(). JDBC 4.2 Table B-5 mandates this conversion for every column type, so handling it ahead of the raw-Object fallback means callers don't depend on getObject() returning a String. 3. Primitive wrapper / BigDecimal / byte[] -> delegate to the matching primitive getter. This covers numeric narrowing (getObject(col, Integer.class) on a BIGINT column) and honours wasNull so a null column returns null instead of an auto-boxed 0. 4. Otherwise raw + isInstance, accepting any supertype/interface match. 5. Anything left throws SQLFeatureNotSupportedException (matching the JDBC spec's expectation for unsupported conversions, and consistent with the existing TimeStampVectorAccessor override). Accessors with richer needs (timestamp accessors return Instant, OffsetDateTime, etc.) keep their overrides; the testGetObjectWithInstantClassUsesOverride test pins that the override path still wins over the new base default. Tests in StreamingResultSetMethodTest cover: - getObjectWithClassUsesAccessorBaseFallback: identity case (VARCHAR -> String) goes through the inherited String fast-path. - getObjectWithSupertypeOrInterfaceReturnsValue: Object.class and CharSequence.class both work via isInstance — proves polymorphic callers (raw Object holders, generic tooling) aren't blocked. - getObjectWithNullClassThrows: null type parameter raises SQLException with the expected "must not be null" message. - getObjectWithIncompatibleClassThrows: requesting an unrelated type (String column, StringBuilder asked) raises the typed conversion error. - getObjectWithClassReturnsNullForNullValue: a null column value short-circuits and returns null regardless of the requested type; wasNull() is true afterwards. TimeStampVectorAccessorTest.testGetObjectWithInstantClassUsesOverride pins that subclass overrides take precedence over the new base fallback.
1 parent 1bda7d4 commit 6817dde

3 files changed

Lines changed: 199 additions & 4 deletions

File tree

jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/accessor/QueryJDBCAccessor.java

Lines changed: 97 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,9 +193,103 @@ public Reader getNCharacterStream() throws SQLException {
193193
throw getOperationNotSupported(this.getClass());
194194
}
195195

196-
@Override
197-
public <T> T getObject(Class<T> aClass) throws SQLException {
198-
throw getOperationNotSupported(aClass.getClass());
196+
/**
197+
* Default {@code getObject(Class)} implementation covering the JDBC 4.2 conversions
198+
* accessible from a {@link Object} or primitive getter.
199+
*
200+
* <p>Resolution order:
201+
* <ol>
202+
* <li>{@code null} type → {@code SQLException} with SQLState {@code 22023}.
203+
* <li>{@link String} → delegate to {@link #getString()} (JDBC 4.2 Table B-5 mandates this
204+
* conversion for every column type).
205+
* <li>Primitive wrapper / {@link java.math.BigDecimal} / {@code byte[]} → delegate to the
206+
* matching primitive getter so numeric narrowing (e.g. {@code Integer.class} on a
207+
* BIGINT column) works. {@link #wasNull()} is honoured: a null column returns
208+
* {@code null} regardless of which wrapper was requested.
209+
* <li>Otherwise, take the raw object from {@link #getObject()} and return it via
210+
* {@code type.isInstance} when it fits.
211+
* <li>Anything left throws {@link SQLFeatureNotSupportedException}.
212+
* </ol>
213+
*
214+
* <p>Accessors that need richer conversion (e.g. an Arrow {@code TIMESTAMP} vector returning
215+
* {@link java.time.Instant} / {@link java.time.OffsetDateTime}) override this; everyone else
216+
* inherits this generic path so callers do not need to special-case "accessor implements
217+
* typed getObject" vs "accessor does not".
218+
*/
219+
@Override
220+
public <T> T getObject(Class<T> type) throws SQLException {
221+
if (type == null) {
222+
throw new SQLException("type parameter must not be null", "22023");
223+
}
224+
225+
// String works on every column type per JDBC 4.2 Table B-5; handle it before the
226+
// raw-object fallback so callers don't depend on getObject() returning a String.
227+
if (type == String.class) {
228+
return type.cast(getString());
229+
}
230+
231+
// Primitive-wrapper and java.math/byte[] conversions: delegate to the matching getter
232+
// and short-circuit to null on the wasNull case so primitive defaults (0, false) don't
233+
// leak as auto-boxed wrappers.
234+
Object value = invokePrimitiveGetter(type);
235+
if (value != PRIMITIVE_GETTER_MISS) {
236+
return wasNull ? null : type.cast(value);
237+
}
238+
239+
// Generic raw-object path: works whenever the column's natural Object representation
240+
// is already an instance of the requested type (identity, supertype, or interface).
241+
// Concrete accessors set wasNull inside getObject(); set it defensively here too so
242+
// a callsite reading a null does not see stale state from an earlier non-null read.
243+
Object raw = getObject();
244+
if (raw == null) {
245+
wasNull = true;
246+
return null;
247+
}
248+
if (type.isInstance(raw)) {
249+
return type.cast(raw);
250+
}
251+
throw new SQLFeatureNotSupportedException(
252+
"Cannot convert column value of type " + raw.getClass().getName() + " to " + type.getName());
253+
}
254+
255+
/** Sentinel returned when {@link #invokePrimitiveGetter} did not match the requested type. */
256+
private static final Object PRIMITIVE_GETTER_MISS = new Object();
257+
258+
/**
259+
* Returns the value of the matching primitive getter for {@code type}, auto-boxed for
260+
* non-Object return types. Returns {@link #PRIMITIVE_GETTER_MISS} (a distinct sentinel from
261+
* {@code null}, which is a legitimate return for {@code byte[].class}) when no primitive
262+
* getter matches, so the caller can fall back to the raw-object path.
263+
*/
264+
private Object invokePrimitiveGetter(Class<?> type) throws SQLException {
265+
if (type == Boolean.class) {
266+
return getBoolean();
267+
}
268+
if (type == Byte.class) {
269+
return getByte();
270+
}
271+
if (type == Short.class) {
272+
return getShort();
273+
}
274+
if (type == Integer.class) {
275+
return getInt();
276+
}
277+
if (type == Long.class) {
278+
return getLong();
279+
}
280+
if (type == Float.class) {
281+
return getFloat();
282+
}
283+
if (type == Double.class) {
284+
return getDouble();
285+
}
286+
if (type == BigDecimal.class) {
287+
return getBigDecimal();
288+
}
289+
if (type == byte[].class) {
290+
return getBytes();
291+
}
292+
return PRIMITIVE_GETTER_MISS;
199293
}
200294

201295
private static SQLException getOperationNotSupported(final Class<?> type) {

jdbc-core/src/test/java/com/salesforce/datacloud/jdbc/core/StreamingResultSetMethodTest.java

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,24 @@ class StreamingResultSetMethodTest {
3838

3939
@SneakyThrows
4040
private StreamingResultSet createResultSet() {
41+
return createSingleVarCharResultSet(false);
42+
}
43+
44+
@SneakyThrows
45+
private StreamingResultSet createResultSetWithNullValue() {
46+
return createSingleVarCharResultSet(true);
47+
}
48+
49+
@SneakyThrows
50+
private StreamingResultSet createSingleVarCharResultSet(boolean nullValue) {
4151
val allocator = ext.getRootAllocator();
4252
val vector = new VarCharVector("col1", allocator);
4353
vector.allocateNew();
44-
vector.set(0, "hello".getBytes(StandardCharsets.UTF_8));
54+
if (nullValue) {
55+
vector.setNull(0);
56+
} else {
57+
vector.set(0, "hello".getBytes(StandardCharsets.UTF_8));
58+
}
4559
vector.setValueCount(1);
4660

4761
val root = new VectorSchemaRoot(Arrays.asList(vector.getField()), Arrays.asList(vector));
@@ -143,6 +157,65 @@ void methodsThrowAfterClose() throws Exception {
143157
.hasMessageContaining("closed");
144158
}
145159

160+
@Test
161+
void getObjectWithClassUsesAccessorBaseFallback() throws Exception {
162+
// VarCharVectorAccessor does not override getObject(Class); it inherits the default in
163+
// QueryJDBCAccessor that does raw + isInstance. Pin that this delivers a String for a
164+
// VARCHAR column — regressing the base-class fallback breaks every accessor that does
165+
// not implement typed conversion of its own.
166+
try (val rs = createResultSet()) {
167+
rs.next();
168+
assertThat(rs.getObject(1, String.class)).isEqualTo("hello");
169+
}
170+
}
171+
172+
@Test
173+
void getObjectWithNullClassThrows() throws Exception {
174+
try (val rs = createResultSet()) {
175+
rs.next();
176+
assertThatThrownBy(() -> rs.getObject(1, (Class<?>) null))
177+
.isInstanceOf(SQLException.class)
178+
.hasMessageContaining("must not be null");
179+
}
180+
}
181+
182+
@Test
183+
void getObjectWithIncompatibleClassThrows() throws Exception {
184+
// VarCharVectorAccessor returns a String. Asking for an unrelated type (StringBuilder
185+
// here) cannot be satisfied by isInstance, so the fallback should surface a typed
186+
// conversion error rather than silently returning null or the raw string.
187+
try (val rs = createResultSet()) {
188+
rs.next();
189+
assertThatThrownBy(() -> rs.getObject(1, StringBuilder.class))
190+
.isInstanceOf(SQLException.class)
191+
.hasMessageContaining("Cannot convert");
192+
}
193+
}
194+
195+
@Test
196+
void getObjectWithClassReturnsNullForNullValue() throws Exception {
197+
// A null column value should round-trip as null regardless of the requested type — the
198+
// fallback short-circuits before the isInstance check.
199+
try (val rs = createResultSetWithNullValue()) {
200+
rs.next();
201+
assertThat(rs.getObject(1, String.class)).isNull();
202+
assertThat(rs.wasNull()).isTrue();
203+
}
204+
}
205+
206+
@Test
207+
void getObjectWithSupertypeOrInterfaceReturnsValue() throws Exception {
208+
// The isInstance check accepts any supertype or interface the raw object implements,
209+
// not just the exact runtime class. Polymorphic callers (e.g. Object.class for
210+
// generic introspection, CharSequence.class for tools that don't care about
211+
// String-vs-StringBuffer) need this to work.
212+
try (val rs = createResultSet()) {
213+
rs.next();
214+
assertThat((String) rs.getObject(1, Object.class)).isEqualTo("hello");
215+
assertThat(rs.getObject(1, CharSequence.class).toString()).isEqualTo("hello");
216+
}
217+
}
218+
146219
@Test
147220
void queryId() throws Exception {
148221
try (val rs = createResultSet()) {

jdbc-core/src/test/java/com/salesforce/datacloud/jdbc/core/accessor/impl/TimeStampVectorAccessorTest.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,34 @@ void testGetObjectWithAllTypesForNaiveTimestamp() {
755755
}
756756
}
757757

758+
/**
759+
* The {@link QueryJDBCAccessor} base class added a default {@code getObject(Class)} that
760+
* handles String, primitive wrappers and the raw-Object isInstance case, but does not know
761+
* about {@link Instant}. Pin that asking for {@code Instant.class} on a TIMESTAMP column
762+
* still goes through this accessor's override (which does know how to build an Instant)
763+
* rather than falling through to the base default — otherwise the override would silently
764+
* dead-code itself if someone reordered method-resolution.
765+
*/
766+
@Test
767+
@SneakyThrows
768+
void testGetObjectWithInstantClassUsesOverride() {
769+
Calendar calendar = Calendar.getInstance();
770+
calendar.setTimeZone(TimeZone.getTimeZone("UTC"));
771+
List<Integer> monthNumber = getRandomMonthNumber();
772+
val values = getMilliSecondValues(calendar, monthNumber);
773+
774+
try (val vector = extension.createTimeStampMilliVector(values)) {
775+
val i = new AtomicInteger(0);
776+
val sut = new TimeStampVectorAccessor(vector, i::get);
777+
778+
// First row: the override produces a real Instant matching the underlying value;
779+
// the base class would have rejected Instant.class with SQLFeatureNotSupportedException.
780+
Instant instant = sut.getObject(Instant.class);
781+
collector.assertThat(instant).isNotNull();
782+
collector.assertThat(instant.toEpochMilli()).isEqualTo(values.get(0));
783+
}
784+
}
785+
758786
@Test
759787
@SneakyThrows
760788
void testGetObjectWithAllTypesForTimestampTZ() {

0 commit comments

Comments
 (0)