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 void set(int index, NullableTimeStampMicroTZHolder 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/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 void set(int index, NullableTimeStampMilliTZHolder 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/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 Long getObject(int index) { 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 Long getObject(int index) { 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.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 void testSetNullableVarCharHolderSafe() { } } + @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)) {