Skip to content

Commit dfbd02f

Browse files
Kaustav-SarkarKaustav-Sarkar
committed
GH-125: Fix Timestamp unset bug
1 parent 8d1802c commit dfbd02f

File tree

5 files changed

+197
-4
lines changed

5 files changed

+197
-4
lines changed

‎vector/src/main/java/org/apache/arrow/vector/TimeStampMicroTZVector.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ public void set(int index, NullableTimeStampMicroTZHolder holder)
155155
throws IllegalArgumentException {
156156
if (holder.isSet < 0) {
157157
throw new IllegalArgumentException();
158-
} else if (!this.timeZone.equals(holder.timezone)) {
158+
} else if (holder.isSet > 0 && !this.timeZone.equals(holder.timezone)) {
159159
throw new IllegalArgumentException(
160160
String.format(
161161
"holder.timezone: %s not equal to vector timezone: %s",

‎vector/src/main/java/org/apache/arrow/vector/TimeStampMilliTZVector.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ public void set(int index, NullableTimeStampMilliTZHolder holder)
155155
throws IllegalArgumentException {
156156
if (holder.isSet < 0) {
157157
throw new IllegalArgumentException();
158-
} else if (!this.timeZone.equals(holder.timezone)) {
158+
} else if (holder.isSet > 0 && !this.timeZone.equals(holder.timezone)) {
159159
throw new IllegalArgumentException(
160160
String.format(
161161
"holder.timezone: %s not equal to vector timezone: %s",

‎vector/src/main/java/org/apache/arrow/vector/TimeStampNanoTZVector.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ public Long getObject(int index) {
154154
public void set(int index, NullableTimeStampNanoTZHolder holder) throws IllegalArgumentException {
155155
if (holder.isSet < 0) {
156156
throw new IllegalArgumentException();
157-
} else if (!this.timeZone.equals(holder.timezone)) {
157+
} else if (holder.isSet > 0 && !this.timeZone.equals(holder.timezone)) {
158158
throw new IllegalArgumentException(
159159
String.format(
160160
"holder.timezone: %s not equal to vector timezone: %s",

‎vector/src/main/java/org/apache/arrow/vector/TimeStampSecTZVector.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ public Long getObject(int index) {
150150
public void set(int index, NullableTimeStampSecTZHolder holder) throws IllegalArgumentException {
151151
if (holder.isSet < 0) {
152152
throw new IllegalArgumentException();
153-
} else if (!this.timeZone.equals(holder.timezone)) {
153+
} else if (holder.isSet > 0 && !this.timeZone.equals(holder.timezone)) {
154154
throw new IllegalArgumentException(
155155
String.format(
156156
"holder.timezone: %s not equal to vector timezone: %s",

‎vector/src/test/java/org/apache/arrow/vector/TestValueVector.java‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@
5757
import org.apache.arrow.vector.complex.impl.UnionListViewWriter;
5858
import org.apache.arrow.vector.complex.impl.UnionListWriter;
5959
import org.apache.arrow.vector.holders.NullableIntHolder;
60+
import org.apache.arrow.vector.holders.NullableTimeStampMicroTZHolder;
61+
import org.apache.arrow.vector.holders.NullableTimeStampMilliTZHolder;
62+
import org.apache.arrow.vector.holders.NullableTimeStampNanoTZHolder;
63+
import org.apache.arrow.vector.holders.NullableTimeStampSecTZHolder;
6064
import org.apache.arrow.vector.holders.NullableUInt4Holder;
6165
import org.apache.arrow.vector.holders.NullableVarBinaryHolder;
6266
import org.apache.arrow.vector.holders.NullableVarCharHolder;
@@ -2567,6 +2571,195 @@ public void testSetNullableVarCharHolderSafe() {
25672571
}
25682572
}
25692573

2574+
@Test
2575+
public void testTimeStampTZVectorSetSafeUnset() {
2576+
// reproduction of https://github.com/apache/arrow/issues/45084
2577+
try (TimeStampMicroTZVector vector = new TimeStampMicroTZVector("vector", allocator, "UTC")) {
2578+
vector.allocateNew();
2579+
// Set a valid value
2580+
NullableTimeStampMicroTZHolder validHolder = new NullableTimeStampMicroTZHolder();
2581+
validHolder.isSet = 1;
2582+
validHolder.value = 1000L;
2583+
validHolder.timezone = "UTC";
2584+
vector.setSafe(0, validHolder);
2585+
2586+
assertEquals(1000L, vector.get(0));
2587+
2588+
// Unset the value using a holder with default (null) timezone
2589+
// The bug used to throw IllegalArgumentException because holder.timezone (null) !=
2590+
// vector.timezone ("UTC")
2591+
// The correct behaviour is to not throw an exception and to unset the value.
2592+
NullableTimeStampMicroTZHolder unsetHolder = new NullableTimeStampMicroTZHolder();
2593+
unsetHolder.isSet = 0;
2594+
vector.setSafe(0, unsetHolder);
2595+
2596+
assertNull(vector.getObject(0));
2597+
}
2598+
}
2599+
2600+
@Test
2601+
public void testTimeStampMilliTZVectorSetSafeUnset() {
2602+
// reproduction of https://github.com/apache/arrow/issues/45084
2603+
try (TimeStampMilliTZVector vector = new TimeStampMilliTZVector("vector", allocator, "UTC")) {
2604+
vector.allocateNew();
2605+
2606+
NullableTimeStampMilliTZHolder validHolder = new NullableTimeStampMilliTZHolder();
2607+
validHolder.isSet = 1;
2608+
validHolder.value = 1000L;
2609+
validHolder.timezone = "UTC";
2610+
vector.setSafe(0, validHolder);
2611+
2612+
assertEquals(1000L, vector.get(0));
2613+
2614+
NullableTimeStampMilliTZHolder unsetHolder = new NullableTimeStampMilliTZHolder();
2615+
unsetHolder.isSet = 0;
2616+
vector.setSafe(0, unsetHolder);
2617+
2618+
assertNull(vector.getObject(0));
2619+
}
2620+
}
2621+
2622+
@Test
2623+
public void testTimeStampNanoTZVectorSetSafeUnset() {
2624+
// reproduction of https://github.com/apache/arrow/issues/45084
2625+
try (TimeStampNanoTZVector vector = new TimeStampNanoTZVector("vector", allocator, "UTC")) {
2626+
vector.allocateNew();
2627+
2628+
NullableTimeStampNanoTZHolder validHolder = new NullableTimeStampNanoTZHolder();
2629+
validHolder.isSet = 1;
2630+
validHolder.value = 1000L;
2631+
validHolder.timezone = "UTC";
2632+
vector.setSafe(0, validHolder);
2633+
2634+
assertEquals(1000L, vector.get(0));
2635+
2636+
NullableTimeStampNanoTZHolder unsetHolder = new NullableTimeStampNanoTZHolder();
2637+
unsetHolder.isSet = 0;
2638+
vector.setSafe(0, unsetHolder);
2639+
2640+
assertNull(vector.getObject(0));
2641+
}
2642+
}
2643+
2644+
@Test
2645+
public void testTimeStampSecTZVectorSetSafeUnset() {
2646+
// reproduction of https://github.com/apache/arrow/issues/45084
2647+
try (TimeStampSecTZVector vector = new TimeStampSecTZVector("vector", allocator, "UTC")) {
2648+
vector.allocateNew();
2649+
2650+
NullableTimeStampSecTZHolder validHolder = new NullableTimeStampSecTZHolder();
2651+
validHolder.isSet = 1;
2652+
validHolder.value = 1000L;
2653+
validHolder.timezone = "UTC";
2654+
vector.setSafe(0, validHolder);
2655+
2656+
assertEquals(1000L, vector.get(0));
2657+
2658+
NullableTimeStampSecTZHolder unsetHolder = new NullableTimeStampSecTZHolder();
2659+
unsetHolder.isSet = 0;
2660+
vector.setSafe(0, unsetHolder);
2661+
2662+
assertNull(vector.getObject(0));
2663+
}
2664+
}
2665+
2666+
@Test
2667+
public void testTimeStampMicroTZVectorSetSafeUnsetExplicitTimezone() {
2668+
// Test to ensure fix added for https://github.com/apache/arrow/issues/45084 does not break
2669+
// workaround.
2670+
try (TimeStampMicroTZVector vector = new TimeStampMicroTZVector("vector", allocator, "UTC")) {
2671+
vector.allocateNew();
2672+
2673+
NullableTimeStampMicroTZHolder validHolder = new NullableTimeStampMicroTZHolder();
2674+
validHolder.isSet = 1;
2675+
validHolder.value = 1000L;
2676+
validHolder.timezone = "UTC";
2677+
vector.setSafe(0, validHolder);
2678+
2679+
assertEquals(1000L, vector.get(0));
2680+
2681+
NullableTimeStampMicroTZHolder unsetHolder = new NullableTimeStampMicroTZHolder();
2682+
unsetHolder.isSet = 0;
2683+
unsetHolder.timezone = "UTC";
2684+
2685+
vector.setSafe(0, unsetHolder);
2686+
2687+
assertNull(vector.getObject(0));
2688+
}
2689+
}
2690+
2691+
@Test
2692+
public void testTimeStampMilliTZVectorSetSafeUnsetExplicitTimezone() {
2693+
// Test to ensure fix added for https://github.com/apache/arrow/issues/45084 does not break
2694+
// workaround.
2695+
try (TimeStampMilliTZVector vector = new TimeStampMilliTZVector("vector", allocator, "UTC")) {
2696+
vector.allocateNew();
2697+
2698+
NullableTimeStampMilliTZHolder validHolder = new NullableTimeStampMilliTZHolder();
2699+
validHolder.isSet = 1;
2700+
validHolder.value = 1000L;
2701+
validHolder.timezone = "UTC";
2702+
vector.setSafe(0, validHolder);
2703+
2704+
assertEquals(1000L, vector.get(0));
2705+
2706+
NullableTimeStampMilliTZHolder unsetHolder = new NullableTimeStampMilliTZHolder();
2707+
unsetHolder.isSet = 0;
2708+
unsetHolder.timezone = "UTC";
2709+
vector.setSafe(0, unsetHolder);
2710+
2711+
assertNull(vector.getObject(0));
2712+
}
2713+
}
2714+
2715+
@Test
2716+
public void testTimeStampNanoTZVectorSetSafeUnsetExplicitTimezone() {
2717+
// Test to ensure fix added for https://github.com/apache/arrow/issues/45084 does not break
2718+
// workaround.
2719+
try (TimeStampNanoTZVector vector = new TimeStampNanoTZVector("vector", allocator, "UTC")) {
2720+
vector.allocateNew();
2721+
2722+
NullableTimeStampNanoTZHolder validHolder = new NullableTimeStampNanoTZHolder();
2723+
validHolder.isSet = 1;
2724+
validHolder.value = 1000L;
2725+
validHolder.timezone = "UTC";
2726+
vector.setSafe(0, validHolder);
2727+
2728+
assertEquals(1000L, vector.get(0));
2729+
2730+
NullableTimeStampNanoTZHolder unsetHolder = new NullableTimeStampNanoTZHolder();
2731+
unsetHolder.isSet = 0;
2732+
unsetHolder.timezone = "UTC";
2733+
vector.setSafe(0, unsetHolder);
2734+
2735+
assertNull(vector.getObject(0));
2736+
}
2737+
}
2738+
2739+
@Test
2740+
public void testTimeStampSecTZVectorSetSafeUnsetExplicitTimezone() {
2741+
// Test to ensure fix added for https://github.com/apache/arrow/issues/45084 does not break
2742+
// workaround.
2743+
try (TimeStampSecTZVector vector = new TimeStampSecTZVector("vector", allocator, "UTC")) {
2744+
vector.allocateNew();
2745+
2746+
NullableTimeStampSecTZHolder validHolder = new NullableTimeStampSecTZHolder();
2747+
validHolder.isSet = 1;
2748+
validHolder.value = 1000L;
2749+
validHolder.timezone = "UTC";
2750+
vector.setSafe(0, validHolder);
2751+
2752+
assertEquals(1000L, vector.get(0));
2753+
2754+
NullableTimeStampSecTZHolder unsetHolder = new NullableTimeStampSecTZHolder();
2755+
unsetHolder.isSet = 0;
2756+
unsetHolder.timezone = "UTC";
2757+
vector.setSafe(0, unsetHolder);
2758+
2759+
assertNull(vector.getObject(0));
2760+
}
2761+
}
2762+
25702763
@Test
25712764
public void testSetNullableVarBinaryHolder() {
25722765
try (VarBinaryVector vector = new VarBinaryVector("", allocator)) {

0 commit comments

Comments
 (0)