Skip to content

Commit f4cad29

Browse files
committed
refactor: trim docstrings and prune redundant ResultSet-shape tests
Three small follow-ups from PR #175 review: - StreamingResultSet.of: drop the paragraph that pointed at the HyperTypeToArrow.JDBC_TYPE_NAME_METADATA_KEY field-metadata key. The docstring spilled implementation detail of the metadata-stamping path into a generic "create a result set from a reader" entry-point; the type-name override is documented at HyperTypeToArrow / ColumnMetadata where it's relevant. - ArrowStreamReaderCursor.loadNextNonEmptyBatch: rewrite the rationale to answer "why does the cursor consume empty batches instead of the caller?" directly. Empty IPC batches are valid Arrow and producers emit them; JDBC's next() only knows rows, so this cursor is the seam that translates batch-level signals into row-level advances. - MetadataResultSetsTest: drop the JDBC ResultSet-shape slice (next / isClosed / getStatement / unwrap / isWrapperFor / getHoldability / getFetchSize / setFetchSize / getWarnings / getConcurrency / getType / getFetchDirection). Those test the StreamingResultSet plumbing shared by every result set on this branch and are already covered by StreamingResultSetMethodTest. Keep the arity-contract slice (short/long/right/null/empty rows) — that is the metadata-result-set-specific behavior. Addresses: review comments on PR #175.
1 parent 1d848b1 commit f4cad29

2 files changed

Lines changed: 5 additions & 107 deletions

File tree

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,6 @@ public static StreamingResultSet of(ArrowStreamReader reader, BufferAllocator al
8787
* buffer accounting clears before the allocator's budget check. Callers must not close
8888
* either separately.
8989
*
90-
* <p>The column metadata (including any {@link ColumnMetadata#getTypeName()} override
91-
* stamped under {@link com.salesforce.datacloud.jdbc.protocol.data.HyperTypeToArrow#JDBC_TYPE_NAME_METADATA_KEY})
92-
* is derived from the Arrow schema via {@link ArrowToHyperTypeMapper#toColumnMetadata(org.apache.arrow.vector.types.pojo.Field)}.
93-
*
9490
* @param reader The Arrow stream, owned by the result set.
9591
* @param allocator The allocator backing the reader, owned by the result set.
9692
* @param queryId The query identifier.

jdbc-core/src/test/java/com/salesforce/datacloud/jdbc/core/metadata/MetadataResultSetsTest.java

Lines changed: 5 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -6,32 +6,21 @@
66

77
import static org.assertj.core.api.Assertions.assertThat;
88
import static org.assertj.core.api.Assertions.assertThatThrownBy;
9-
import static org.junit.jupiter.api.Assertions.assertFalse;
10-
import static org.junit.jupiter.api.Assertions.assertThrows;
11-
import static org.junit.jupiter.api.Assertions.assertTrue;
129

13-
import com.salesforce.datacloud.jdbc.core.MetadataSchemas;
1410
import com.salesforce.datacloud.jdbc.protocol.data.ColumnMetadata;
1511
import com.salesforce.datacloud.jdbc.protocol.data.HyperType;
16-
import java.sql.ResultSet;
17-
import java.sql.ResultSetMetaData;
18-
import java.sql.SQLException;
1912
import java.util.Arrays;
2013
import java.util.Collections;
2114
import java.util.List;
22-
import lombok.SneakyThrows;
2315
import lombok.val;
24-
import org.junit.jupiter.api.BeforeEach;
2516
import org.junit.jupiter.api.Test;
2617

2718
/**
28-
* Tests for {@link MetadataResultSets}. Two slices:
29-
* <ul>
30-
* <li><b>Arity contract</b> — rows must match the schema column count; null rows are allowed
31-
* as the all-nulls shape (matching the legacy {@code coerceRows} convention).
32-
* <li><b>JDBC ResultSet shape</b> — empty metadata result sets expose the standard JDBC shape
33-
* (row=0, closeable, forward-only, holdability, etc.).
34-
* </ul>
19+
* Tests the {@link MetadataResultSets#of} arity contract: rows must match the schema column
20+
* count; null rows are allowed as the all-nulls shape (matching the legacy {@code coerceRows}
21+
* convention). Generic JDBC {@link java.sql.ResultSet} shape (closeable, forward-only,
22+
* holdability, etc.) is exercised by {@code StreamingResultSetMethodTest} since metadata
23+
* result sets share the {@link com.salesforce.datacloud.jdbc.core.StreamingResultSet} plumbing.
3524
*/
3625
class MetadataResultSetsTest {
3726

@@ -40,8 +29,6 @@ class MetadataResultSetsTest {
4029
new ColumnMetadata("b", HyperType.int32(true)),
4130
new ColumnMetadata("c", HyperType.bool(true)));
4231

43-
// --- Arity contract ---
44-
4532
@Test
4633
void shortRowRejected() {
4734
val rows = Collections.singletonList(Arrays.<Object>asList("only-one"));
@@ -88,89 +75,4 @@ void emptyRowsAccepted() throws Exception {
8875
assertThat(rs.next()).isFalse();
8976
}
9077
}
91-
92-
// --- JDBC ResultSet shape on an empty metadata result set ---
93-
94-
private ResultSet emptyMetadataResultSet;
95-
96-
@BeforeEach
97-
public void initEmptyMetadataResultSet() throws SQLException {
98-
emptyMetadataResultSet = MetadataResultSets.empty(MetadataSchemas.COLUMNS);
99-
}
100-
101-
@Test
102-
void getRow() throws SQLException {
103-
assertThat(emptyMetadataResultSet.getRow()).isEqualTo(0);
104-
105-
emptyMetadataResultSet.close();
106-
assertThrows(SQLException.class, () -> emptyMetadataResultSet.next());
107-
}
108-
109-
@Test
110-
void next() throws SQLException {
111-
emptyMetadataResultSet.close();
112-
assertThrows(SQLException.class, () -> emptyMetadataResultSet.next());
113-
}
114-
115-
@Test
116-
void isClosed() throws SQLException {
117-
assertFalse(emptyMetadataResultSet.isClosed());
118-
emptyMetadataResultSet.close();
119-
assertTrue(emptyMetadataResultSet.isClosed());
120-
}
121-
122-
@Test
123-
void getStatement() throws SQLException {
124-
assertThat(emptyMetadataResultSet.getStatement()).isNull();
125-
}
126-
127-
@Test
128-
void unwrap() {
129-
assertThrows(SQLException.class, () -> emptyMetadataResultSet.unwrap(ResultSetMetaData.class));
130-
}
131-
132-
@Test
133-
void isWrapperFor() throws SQLException {
134-
// StreamingResultSet implements DataCloudResultSet / ResultSet; it is not a wrapper for
135-
// arbitrary unrelated types.
136-
assertThat(emptyMetadataResultSet.isWrapperFor(ResultSetMetaData.class)).isFalse();
137-
}
138-
139-
@Test
140-
void getHoldability() throws SQLException {
141-
assertThat(emptyMetadataResultSet.getHoldability()).isEqualTo(ResultSet.HOLD_CURSORS_OVER_COMMIT);
142-
}
143-
144-
@Test
145-
void getFetchSize() throws SQLException {
146-
assertThat(emptyMetadataResultSet.getFetchSize()).isEqualTo(0);
147-
}
148-
149-
@Test
150-
void setFetchSize() throws SQLException {
151-
// StreamingResultSet controls its own fetch size and ignores caller-supplied hints.
152-
emptyMetadataResultSet.setFetchSize(0);
153-
}
154-
155-
@SneakyThrows
156-
@Test
157-
void getWarnings() {
158-
assertThat((Iterable<? extends Throwable>) emptyMetadataResultSet.getWarnings())
159-
.isNull();
160-
}
161-
162-
@Test
163-
void getConcurrency() throws SQLException {
164-
assertThat(emptyMetadataResultSet.getConcurrency()).isEqualTo(ResultSet.CONCUR_READ_ONLY);
165-
}
166-
167-
@Test
168-
void getType() throws SQLException {
169-
assertThat(emptyMetadataResultSet.getType()).isEqualTo(ResultSet.TYPE_FORWARD_ONLY);
170-
}
171-
172-
@Test
173-
void getFetchDirection() throws SQLException {
174-
assertThat(emptyMetadataResultSet.getFetchDirection()).isEqualTo(ResultSet.FETCH_FORWARD);
175-
}
17678
}

0 commit comments

Comments
 (0)