Skip to content

Commit 9f0b7c4

Browse files
committed
fix: validate row arity in MetadataResultSets.of
Previously a row with the wrong number of elements would silently leave the trailing columns as Arrow null (interpreted as missing values). Today every caller routes through MetadataSchemas so the sizes match by construction, but a future caller bug would surface only inside vector population, far from the boundary. Add an explicit arity check at the of(...) entrypoint: each non-null row must have exactly columns.size() elements. Null rows are accepted as the all-nulls row (matching the legacy coerceRows convention of turning null into emptyList). Empty rows are accepted only when the schema is also empty. Pin behavior with MetadataResultSetsTest covering short, long, correct-arity, null-row, and empty-rows cases.
1 parent 30dffe8 commit 9f0b7c4

2 files changed

Lines changed: 108 additions & 1 deletion

File tree

jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/metadata/MetadataResultSets.java

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,14 @@ public static StreamingResultSet emptyNoColumns() throws SQLException {
5050

5151
/**
5252
* Build a result set whose schema is {@code columns} and whose rows are {@code rows}. Each
53-
* inner list in {@code rows} supplies values in column order.
53+
* inner list in {@code rows} supplies values in column order, and must have exactly
54+
* {@code columns.size()} elements — a short row would silently leave the trailing columns
55+
* unset (interpreted as Arrow null), which is almost always a caller bug. Today every caller
56+
* goes through {@link MetadataSchemas} so the sizes match by construction; the precondition
57+
* here makes a future caller bug surface at the boundary instead of in vector population.
5458
*/
5559
public static StreamingResultSet of(List<ColumnMetadata> columns, List<List<Object>> rows) throws SQLException {
60+
validateRowArity(columns, rows);
5661
byte[] ipcBytes = writeArrowStream(columns, rows);
5762
// Allocator is handed to StreamingResultSet along with the reader; the result set owns
5863
// its lifecycle and closes it when close() is called.
@@ -105,6 +110,31 @@ private static byte[] writeArrowStream(List<ColumnMetadata> columns, List<List<O
105110
}
106111
}
107112

113+
/**
114+
* Verify that every supplied row has exactly {@code columns.size()} elements. A {@code null}
115+
* row is allowed and is interpreted as a row of all-nulls (matching the old
116+
* {@code coerceRows} convention of converting null rows to empty lists, which is the only
117+
* shape with no positional values to populate). Anything else is a caller bug.
118+
*/
119+
private static void validateRowArity(List<ColumnMetadata> columns, List<List<Object>> rows) throws SQLException {
120+
int expected = columns.size();
121+
for (int i = 0; i < rows.size(); i++) {
122+
List<Object> row = rows.get(i);
123+
if (row == null) {
124+
continue;
125+
}
126+
// The legacy coerceRows path turns a null-row into Collections.emptyList(); accept
127+
// empty as the "all nulls" shape here too.
128+
if (row.isEmpty() && expected > 0) {
129+
continue;
130+
}
131+
if (row.size() != expected) {
132+
throw new SQLException("Metadata row " + i + " has " + row.size() + " elements but schema has "
133+
+ expected + " columns");
134+
}
135+
}
136+
}
137+
108138
@SuppressWarnings("unchecked")
109139
private static List<List<Object>> coerceRows(List<Object> rawRows) throws SQLException {
110140
if (rawRows == null || rawRows.isEmpty()) {
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/**
2+
* This file is part of https://github.com/forcedotcom/datacloud-jdbc which is released under the
3+
* Apache 2.0 license. See https://github.com/forcedotcom/datacloud-jdbc/blob/main/LICENSE.txt
4+
*/
5+
package com.salesforce.datacloud.jdbc.core.metadata;
6+
7+
import static org.assertj.core.api.Assertions.assertThat;
8+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
9+
10+
import com.salesforce.datacloud.jdbc.protocol.data.ColumnMetadata;
11+
import com.salesforce.datacloud.jdbc.protocol.data.HyperType;
12+
import java.util.Arrays;
13+
import java.util.Collections;
14+
import java.util.List;
15+
import lombok.val;
16+
import org.junit.jupiter.api.Test;
17+
18+
/**
19+
* Pin the {@link MetadataResultSets#of(List, List)} arity contract: rows must match the schema
20+
* column count, otherwise a short row would silently produce trailing Arrow-null cells (almost
21+
* always a caller bug). A {@code null} row is allowed and is interpreted as an all-nulls row,
22+
* matching the legacy {@code coerceRows} convention.
23+
*/
24+
class MetadataResultSetsTest {
25+
26+
private static final List<ColumnMetadata> THREE_COLUMNS = Arrays.asList(
27+
new ColumnMetadata("a", HyperType.varcharUnlimited(true)),
28+
new ColumnMetadata("b", HyperType.int32(true)),
29+
new ColumnMetadata("c", HyperType.bool(true)));
30+
31+
@Test
32+
void shortRowRejected() {
33+
val rows = Collections.singletonList(Arrays.<Object>asList("only-one"));
34+
assertThatThrownBy(() -> MetadataResultSets.of(THREE_COLUMNS, rows))
35+
.hasMessageContaining("3 columns")
36+
.hasMessageContaining("1 elements");
37+
}
38+
39+
@Test
40+
void longRowRejected() {
41+
val rows = Collections.singletonList(Arrays.<Object>asList("a", 1, true, "extra"));
42+
assertThatThrownBy(() -> MetadataResultSets.of(THREE_COLUMNS, rows))
43+
.hasMessageContaining("3 columns")
44+
.hasMessageContaining("4 elements");
45+
}
46+
47+
@Test
48+
void rightArityAccepted() throws Exception {
49+
val rows = Collections.singletonList(Arrays.<Object>asList("a", 1, true));
50+
try (val rs = MetadataResultSets.of(THREE_COLUMNS, rows)) {
51+
assertThat(rs.next()).isTrue();
52+
assertThat(rs.getString(1)).isEqualTo("a");
53+
assertThat(rs.getInt(2)).isEqualTo(1);
54+
assertThat(rs.getBoolean(3)).isTrue();
55+
}
56+
}
57+
58+
@Test
59+
void nullRowAcceptedAsAllNulls() throws Exception {
60+
val rows = Collections.<List<Object>>singletonList(null);
61+
try (val rs = MetadataResultSets.of(THREE_COLUMNS, rows)) {
62+
assertThat(rs.next()).isTrue();
63+
assertThat(rs.getString(1)).isNull();
64+
rs.getInt(2);
65+
assertThat(rs.wasNull()).isTrue();
66+
rs.getBoolean(3);
67+
assertThat(rs.wasNull()).isTrue();
68+
}
69+
}
70+
71+
@Test
72+
void emptyRowsAccepted() throws Exception {
73+
try (val rs = MetadataResultSets.of(THREE_COLUMNS, Collections.emptyList())) {
74+
assertThat(rs.next()).isFalse();
75+
}
76+
}
77+
}

0 commit comments

Comments
 (0)