This is an automated email from the ASF dual-hosted git repository.
jbonofre pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-java.git
The following commit(s) were added to refs/heads/main by this push:
new ccaac9ad6 GH-125: Allow null timestamp holder sans timezone (#941)
ccaac9ad6 is described below
commit ccaac9ad688265d272d4c3d9d824426ef7681669
Author: Kaustav Sarkar <[email protected]>
AuthorDate: Thu Jan 22 23:26:49 2026 +0530
GH-125: Allow null timestamp holder sans timezone (#941)
## Description
Fixes an `IllegalArgumentException` in `TimeStamp*TZVector.set/setSafe`
when unsetting values using a holder with a `null` timezone. The
validation logic now correctly ignores the timezone check when
`holder.isSet <= 0`, allowing default-constructed holders to be used for
unsetting values as expected.
Comprehensive tests added for all timestamp precisions (Micro, Milli,
Nano, Sec) to verify the fix and ensure the existing workaround (setting
explicit timezone) remains supported.
Closes #125 .
---
.../arrow/vector/TimeStampMicroTZVector.java | 11 +-
.../arrow/vector/TimeStampMilliTZVector.java | 11 +-
.../apache/arrow/vector/TimeStampNanoTZVector.java | 11 +-
.../apache/arrow/vector/TimeStampSecTZVector.java | 11 +-
.../org/apache/arrow/vector/TestValueVector.java | 193 +++++++++++++++++++++
5 files changed, 217 insertions(+), 20 deletions(-)
diff --git
a/vector/src/main/java/org/apache/arrow/vector/TimeStampMicroTZVector.java
b/vector/src/main/java/org/apache/arrow/vector/TimeStampMicroTZVector.java
index abaefcfc1..50f2f066c 100644
--- a/vector/src/main/java/org/apache/arrow/vector/TimeStampMicroTZVector.java
+++ b/vector/src/main/java/org/apache/arrow/vector/TimeStampMicroTZVector.java
@@ -155,12 +155,13 @@ public final class TimeStampMicroTZVector extends
TimeStampVector
throws IllegalArgumentException {
if (holder.isSet < 0) {
throw new IllegalArgumentException();
- } else if (!this.timeZone.equals(holder.timezone)) {
- throw new IllegalArgumentException(
- String.format(
- "holder.timezone: %s not equal to vector timezone: %s",
- holder.timezone, this.timeZone));
} else if (holder.isSet > 0) {
+ if (!this.timeZone.equals(holder.timezone)) {
+ throw new IllegalArgumentException(
+ String.format(
+ "holder.timezone: %s not equal to vector timezone: %s",
+ holder.timezone, this.timeZone));
+ }
BitVectorHelper.setBit(validityBuffer, index);
setValue(index, holder.value);
} else {
diff --git
a/vector/src/main/java/org/apache/arrow/vector/TimeStampMilliTZVector.java
b/vector/src/main/java/org/apache/arrow/vector/TimeStampMilliTZVector.java
index b5e5fb1be..9e4998396 100644
--- a/vector/src/main/java/org/apache/arrow/vector/TimeStampMilliTZVector.java
+++ b/vector/src/main/java/org/apache/arrow/vector/TimeStampMilliTZVector.java
@@ -155,12 +155,13 @@ public final class TimeStampMilliTZVector extends
TimeStampVector
throws IllegalArgumentException {
if (holder.isSet < 0) {
throw new IllegalArgumentException();
- } else if (!this.timeZone.equals(holder.timezone)) {
- throw new IllegalArgumentException(
- String.format(
- "holder.timezone: %s not equal to vector timezone: %s",
- holder.timezone, this.timeZone));
} else if (holder.isSet > 0) {
+ if (!this.timeZone.equals(holder.timezone)) {
+ throw new IllegalArgumentException(
+ String.format(
+ "holder.timezone: %s not equal to vector timezone: %s",
+ holder.timezone, this.timeZone));
+ }
BitVectorHelper.setBit(validityBuffer, index);
setValue(index, holder.value);
} else {
diff --git
a/vector/src/main/java/org/apache/arrow/vector/TimeStampNanoTZVector.java
b/vector/src/main/java/org/apache/arrow/vector/TimeStampNanoTZVector.java
index 2386b3a85..b44b3da8d 100644
--- a/vector/src/main/java/org/apache/arrow/vector/TimeStampNanoTZVector.java
+++ b/vector/src/main/java/org/apache/arrow/vector/TimeStampNanoTZVector.java
@@ -154,12 +154,13 @@ public final class TimeStampNanoTZVector extends
TimeStampVector
public void set(int index, NullableTimeStampNanoTZHolder holder) throws
IllegalArgumentException {
if (holder.isSet < 0) {
throw new IllegalArgumentException();
- } else if (!this.timeZone.equals(holder.timezone)) {
- throw new IllegalArgumentException(
- String.format(
- "holder.timezone: %s not equal to vector timezone: %s",
- holder.timezone, this.timeZone));
} else if (holder.isSet > 0) {
+ if (!this.timeZone.equals(holder.timezone)) {
+ throw new IllegalArgumentException(
+ String.format(
+ "holder.timezone: %s not equal to vector timezone: %s",
+ holder.timezone, this.timeZone));
+ }
BitVectorHelper.setBit(validityBuffer, index);
setValue(index, holder.value);
} else {
diff --git
a/vector/src/main/java/org/apache/arrow/vector/TimeStampSecTZVector.java
b/vector/src/main/java/org/apache/arrow/vector/TimeStampSecTZVector.java
index f1774f270..a64a87f69 100644
--- a/vector/src/main/java/org/apache/arrow/vector/TimeStampSecTZVector.java
+++ b/vector/src/main/java/org/apache/arrow/vector/TimeStampSecTZVector.java
@@ -150,12 +150,13 @@ public final class TimeStampSecTZVector extends
TimeStampVector
public void set(int index, NullableTimeStampSecTZHolder holder) throws
IllegalArgumentException {
if (holder.isSet < 0) {
throw new IllegalArgumentException();
- } else if (!this.timeZone.equals(holder.timezone)) {
- throw new IllegalArgumentException(
- String.format(
- "holder.timezone: %s not equal to vector timezone: %s",
- holder.timezone, this.timeZone));
} else if (holder.isSet > 0) {
+ if (!this.timeZone.equals(holder.timezone)) {
+ throw new IllegalArgumentException(
+ String.format(
+ "holder.timezone: %s not equal to vector timezone: %s",
+ holder.timezone, this.timeZone));
+ }
BitVectorHelper.setBit(validityBuffer, index);
setValue(index, holder.value);
} else {
diff --git a/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java
b/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java
index ac8224667..df42d04e6 100644
--- a/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java
+++ b/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java
@@ -57,6 +57,10 @@ import
org.apache.arrow.vector.complex.impl.UnionLargeListViewWriter;
import org.apache.arrow.vector.complex.impl.UnionListViewWriter;
import org.apache.arrow.vector.complex.impl.UnionListWriter;
import org.apache.arrow.vector.holders.NullableIntHolder;
+import org.apache.arrow.vector.holders.NullableTimeStampMicroTZHolder;
+import org.apache.arrow.vector.holders.NullableTimeStampMilliTZHolder;
+import org.apache.arrow.vector.holders.NullableTimeStampNanoTZHolder;
+import org.apache.arrow.vector.holders.NullableTimeStampSecTZHolder;
import org.apache.arrow.vector.holders.NullableUInt4Holder;
import org.apache.arrow.vector.holders.NullableVarBinaryHolder;
import org.apache.arrow.vector.holders.NullableVarCharHolder;
@@ -2567,6 +2571,195 @@ public class TestValueVector {
}
}
+ @Test
+ public void testTimeStampTZVectorSetSafeUnset() {
+ // reproduction of https://github.com/apache/arrow/issues/45084
+ try (TimeStampMicroTZVector vector = new TimeStampMicroTZVector("vector",
allocator, "UTC")) {
+ vector.allocateNew();
+ // Set a valid value
+ NullableTimeStampMicroTZHolder validHolder = new
NullableTimeStampMicroTZHolder();
+ validHolder.isSet = 1;
+ validHolder.value = 1000L;
+ validHolder.timezone = "UTC";
+ vector.setSafe(0, validHolder);
+
+ assertEquals(1000L, vector.get(0));
+
+ // Unset the value using a holder with default (null) timezone
+ // The bug used to throw IllegalArgumentException because
holder.timezone (null) !=
+ // vector.timezone ("UTC")
+ // The correct behaviour is to not throw an exception and to unset the
value.
+ NullableTimeStampMicroTZHolder unsetHolder = new
NullableTimeStampMicroTZHolder();
+ unsetHolder.isSet = 0;
+ vector.setSafe(0, unsetHolder);
+
+ assertNull(vector.getObject(0));
+ }
+ }
+
+ @Test
+ public void testTimeStampMilliTZVectorSetSafeUnset() {
+ // reproduction of https://github.com/apache/arrow/issues/45084
+ try (TimeStampMilliTZVector vector = new TimeStampMilliTZVector("vector",
allocator, "UTC")) {
+ vector.allocateNew();
+
+ NullableTimeStampMilliTZHolder validHolder = new
NullableTimeStampMilliTZHolder();
+ validHolder.isSet = 1;
+ validHolder.value = 1000L;
+ validHolder.timezone = "UTC";
+ vector.setSafe(0, validHolder);
+
+ assertEquals(1000L, vector.get(0));
+
+ NullableTimeStampMilliTZHolder unsetHolder = new
NullableTimeStampMilliTZHolder();
+ unsetHolder.isSet = 0;
+ vector.setSafe(0, unsetHolder);
+
+ assertNull(vector.getObject(0));
+ }
+ }
+
+ @Test
+ public void testTimeStampNanoTZVectorSetSafeUnset() {
+ // reproduction of https://github.com/apache/arrow/issues/45084
+ try (TimeStampNanoTZVector vector = new TimeStampNanoTZVector("vector",
allocator, "UTC")) {
+ vector.allocateNew();
+
+ NullableTimeStampNanoTZHolder validHolder = new
NullableTimeStampNanoTZHolder();
+ validHolder.isSet = 1;
+ validHolder.value = 1000L;
+ validHolder.timezone = "UTC";
+ vector.setSafe(0, validHolder);
+
+ assertEquals(1000L, vector.get(0));
+
+ NullableTimeStampNanoTZHolder unsetHolder = new
NullableTimeStampNanoTZHolder();
+ unsetHolder.isSet = 0;
+ vector.setSafe(0, unsetHolder);
+
+ assertNull(vector.getObject(0));
+ }
+ }
+
+ @Test
+ public void testTimeStampSecTZVectorSetSafeUnset() {
+ // reproduction of https://github.com/apache/arrow/issues/45084
+ try (TimeStampSecTZVector vector = new TimeStampSecTZVector("vector",
allocator, "UTC")) {
+ vector.allocateNew();
+
+ NullableTimeStampSecTZHolder validHolder = new
NullableTimeStampSecTZHolder();
+ validHolder.isSet = 1;
+ validHolder.value = 1000L;
+ validHolder.timezone = "UTC";
+ vector.setSafe(0, validHolder);
+
+ assertEquals(1000L, vector.get(0));
+
+ NullableTimeStampSecTZHolder unsetHolder = new
NullableTimeStampSecTZHolder();
+ unsetHolder.isSet = 0;
+ vector.setSafe(0, unsetHolder);
+
+ assertNull(vector.getObject(0));
+ }
+ }
+
+ @Test
+ public void testTimeStampMicroTZVectorSetSafeUnsetExplicitTimezone() {
+ // Test to ensure fix added for
https://github.com/apache/arrow/issues/45084 does not break
+ // workaround.
+ try (TimeStampMicroTZVector vector = new TimeStampMicroTZVector("vector",
allocator, "UTC")) {
+ vector.allocateNew();
+
+ NullableTimeStampMicroTZHolder validHolder = new
NullableTimeStampMicroTZHolder();
+ validHolder.isSet = 1;
+ validHolder.value = 1000L;
+ validHolder.timezone = "UTC";
+ vector.setSafe(0, validHolder);
+
+ assertEquals(1000L, vector.get(0));
+
+ NullableTimeStampMicroTZHolder unsetHolder = new
NullableTimeStampMicroTZHolder();
+ unsetHolder.isSet = 0;
+ unsetHolder.timezone = "UTC";
+
+ vector.setSafe(0, unsetHolder);
+
+ assertNull(vector.getObject(0));
+ }
+ }
+
+ @Test
+ public void testTimeStampMilliTZVectorSetSafeUnsetExplicitTimezone() {
+ // Test to ensure fix added for
https://github.com/apache/arrow/issues/45084 does not break
+ // workaround.
+ try (TimeStampMilliTZVector vector = new TimeStampMilliTZVector("vector",
allocator, "UTC")) {
+ vector.allocateNew();
+
+ NullableTimeStampMilliTZHolder validHolder = new
NullableTimeStampMilliTZHolder();
+ validHolder.isSet = 1;
+ validHolder.value = 1000L;
+ validHolder.timezone = "UTC";
+ vector.setSafe(0, validHolder);
+
+ assertEquals(1000L, vector.get(0));
+
+ NullableTimeStampMilliTZHolder unsetHolder = new
NullableTimeStampMilliTZHolder();
+ unsetHolder.isSet = 0;
+ unsetHolder.timezone = "UTC";
+ vector.setSafe(0, unsetHolder);
+
+ assertNull(vector.getObject(0));
+ }
+ }
+
+ @Test
+ public void testTimeStampNanoTZVectorSetSafeUnsetExplicitTimezone() {
+ // Test to ensure fix added for
https://github.com/apache/arrow/issues/45084 does not break
+ // workaround.
+ try (TimeStampNanoTZVector vector = new TimeStampNanoTZVector("vector",
allocator, "UTC")) {
+ vector.allocateNew();
+
+ NullableTimeStampNanoTZHolder validHolder = new
NullableTimeStampNanoTZHolder();
+ validHolder.isSet = 1;
+ validHolder.value = 1000L;
+ validHolder.timezone = "UTC";
+ vector.setSafe(0, validHolder);
+
+ assertEquals(1000L, vector.get(0));
+
+ NullableTimeStampNanoTZHolder unsetHolder = new
NullableTimeStampNanoTZHolder();
+ unsetHolder.isSet = 0;
+ unsetHolder.timezone = "UTC";
+ vector.setSafe(0, unsetHolder);
+
+ assertNull(vector.getObject(0));
+ }
+ }
+
+ @Test
+ public void testTimeStampSecTZVectorSetSafeUnsetExplicitTimezone() {
+ // Test to ensure fix added for
https://github.com/apache/arrow/issues/45084 does not break
+ // workaround.
+ try (TimeStampSecTZVector vector = new TimeStampSecTZVector("vector",
allocator, "UTC")) {
+ vector.allocateNew();
+
+ NullableTimeStampSecTZHolder validHolder = new
NullableTimeStampSecTZHolder();
+ validHolder.isSet = 1;
+ validHolder.value = 1000L;
+ validHolder.timezone = "UTC";
+ vector.setSafe(0, validHolder);
+
+ assertEquals(1000L, vector.get(0));
+
+ NullableTimeStampSecTZHolder unsetHolder = new
NullableTimeStampSecTZHolder();
+ unsetHolder.isSet = 0;
+ unsetHolder.timezone = "UTC";
+ vector.setSafe(0, unsetHolder);
+
+ assertNull(vector.getObject(0));
+ }
+ }
+
@Test
public void testSetNullableVarBinaryHolder() {
try (VarBinaryVector vector = new VarBinaryVector("", allocator)) {
