Skip to content

Commit 5e4934e

Browse files
authored
[PECOBLR-421] Add explicit null check for empty values in an Arrow value vector (#827)
* Add null check in arrow value conversion
1 parent eaef770 commit 5e4934e

3 files changed

Lines changed: 89 additions & 0 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
### Fixed
1414
- Fix: unsupported data types in `setObject(int,Object,int targetSqlType)` method in PreparedStatement
15+
- Fix: Added explicit null check for Arrow value vector when the value is empty, and Arrow null checking is disabled.
1516

1617
---
1718
*Note: When making changes, please add your change under the appropriate section with a brief description.*

src/main/java/com/databricks/jdbc/api/impl/converters/ArrowToJavaObjectConverter.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ public static Object convert(
6363
ColumnInfoTypeName requiredType,
6464
String arrowMetadata)
6565
throws DatabricksSQLException {
66+
// check isNull before getting the object from the vector
67+
if (columnVector.isNull(vectorIndex)) {
68+
return null;
69+
}
6670
Object object = columnVector.getObject(vectorIndex);
6771
if (arrowMetadata != null) {
6872
if (arrowMetadata.startsWith(ARRAY)) {

src/test/java/com/databricks/jdbc/api/impl/converters/ArrowToJavaObjectConverterTest.java

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.databricks.jdbc.api.impl.converters;
22

3+
import static com.databricks.jdbc.api.impl.converters.ArrowToJavaObjectConverter.convert;
34
import static com.databricks.jdbc.api.impl.converters.ArrowToJavaObjectConverter.getZoneIdFromTimeZoneOpt;
45
import static com.databricks.jdbc.common.util.DatabricksTypeUtil.VARIANT;
56
import static org.junit.jupiter.api.Assertions.*;
@@ -43,6 +44,89 @@ public void testNullObjectConversion() throws SQLException {
4344
assertNull(convertedObject);
4445
}
4546

47+
@Test
48+
public void testNullHandlingInVarCharVector() throws Exception {
49+
// Create a VarCharVector with 3 values: non-null, null, and empty string
50+
disableArrowNullChecking();
51+
VarCharVector vector = new VarCharVector("varCharVector", this.bufferAllocator);
52+
vector.allocateNew(4);
53+
54+
// Set first value: "hello"
55+
vector.set(0, "hello".getBytes());
56+
57+
// Second value: null (don't set it, which makes it null by default)
58+
59+
// Third value: empty string (explicitly set as "")
60+
vector.set(2, "".getBytes());
61+
62+
// Fourth value: set to null
63+
vector.setNull(3);
64+
65+
// Set vector value count
66+
vector.setValueCount(4);
67+
68+
// Case 1: Non-null value
69+
assertFalse(vector.isNull(0));
70+
assertEquals("hello", convert(vector, 0, ColumnInfoTypeName.STRING, "STRING"));
71+
72+
// Case 2: Null value
73+
assertTrue(vector.isNull(1));
74+
assertNull(convert(vector, 1, ColumnInfoTypeName.STRING, "STRING"));
75+
76+
// Case 3: Empty string (should not be treated as null)
77+
assertFalse(vector.isNull(2));
78+
assertEquals(
79+
"",
80+
convert(
81+
vector,
82+
2,
83+
ColumnInfoTypeName.STRING,
84+
"STRING")); // Empty string should be empty, not null
85+
86+
// Case 4: Explicitly set to null
87+
assertTrue(vector.isNull(3));
88+
String valueWithoutCheck = (String) convert(vector, 3, ColumnInfoTypeName.STRING, "STRING");
89+
// This assertion is expected to fail - it shows the problem when isNull check is removed
90+
assertNull(valueWithoutCheck);
91+
92+
enableArrowNullChecking();
93+
}
94+
95+
private void disableArrowNullChecking() {
96+
System.setProperty("arrow.enable_null_check_for_get", "false");
97+
}
98+
99+
private void enableArrowNullChecking() {
100+
System.setProperty("arrow.enable_null_check_for_get", "true");
101+
}
102+
103+
@Test
104+
public void testByteVectorWithNullChecks() throws Exception {
105+
TinyIntVector vector = new TinyIntVector("tinyIntVector", this.bufferAllocator);
106+
vector.allocateNew(3);
107+
108+
// First value: explicitly set to null
109+
vector.setNull(0);
110+
111+
// Second value: skip setting it, which makes it null by default
112+
113+
// Third value: set to 0
114+
vector.set(2, 0);
115+
116+
vector.setValueCount(3);
117+
118+
// Test our converter with proper null handling
119+
assertTrue(vector.isNull(0));
120+
assertNull(convert(vector, 0, ColumnInfoTypeName.BYTE, "BYTE"));
121+
122+
assertTrue(vector.isNull(1));
123+
assertNull(convert(vector, 1, ColumnInfoTypeName.BYTE, "BYTE"));
124+
125+
// The zero value should still be correctly identified as 0, not null
126+
assertFalse(vector.isNull(2));
127+
assertEquals((byte) 0, convert(vector, 2, ColumnInfoTypeName.BYTE, "BYTE"));
128+
}
129+
46130
@Test
47131
public void testByteConversion() throws SQLException {
48132
TinyIntVector tinyIntVector = new TinyIntVector("tinyIntVector", this.bufferAllocator);

0 commit comments

Comments
 (0)