emkornfield commented on a change in pull request #10177:
URL: https://github.com/apache/arrow/pull/10177#discussion_r690515104



##########
File path: cpp/src/arrow/type.h
##########
@@ -1317,6 +1317,40 @@ class ARROW_EXPORT DayTimeIntervalType : public 
IntervalType {
   std::string name() const override { return "day_time_interval"; }
 };
 
+/// \brief Represents a number of days and milliseconds (fraction of day).
+class ARROW_EXPORT MonthDayNanoIntervalType : public IntervalType {
+ public:
+  struct MonthDayNanos {
+    int32_t months;
+    int32_t days;
+    int64_t nanoseconds;
+    bool operator==(MonthDayNanos other) const {
+      return this->months == other.months && this->days == other.days &&
+             this->nanoseconds == other.nanoseconds;
+    }
+    bool operator!=(MonthDayNanos other) const { return !(*this == other); }
+  };
+  using c_type = MonthDayNanos;
+  using PhysicalType = MonthDayNanoIntervalType;
+
+  static_assert(sizeof(MonthDayNanos) == 16,
+                "MonthDayNanos  struct assumed to be of size 8 bytes");

Review comment:
       done.

##########
File path: cpp/src/arrow/array/array_primitive.h
##########
@@ -132,4 +136,33 @@ class ARROW_EXPORT DayTimeIntervalArray : public 
PrimitiveArray {
   const uint8_t* raw_values() const { return raw_values_ + data_->offset * 
byte_width(); }
 };
 
+/// \brief Array of Month, Day and nanosecond values.
+class ARROW_EXPORT MonthDayNanoIntervalArray : public PrimitiveArray {
+ public:
+  using TypeClass = MonthDayNanoIntervalType;
+
+  explicit MonthDayNanoIntervalArray(const std::shared_ptr<ArrayData>& data);
+
+  MonthDayNanoIntervalArray(const std::shared_ptr<DataType>& type, int64_t 
length,
+                            const std::shared_ptr<Buffer>& data,
+                            const std::shared_ptr<Buffer>& null_bitmap = 
NULLPTR,
+                            int64_t null_count = kUnknownNullCount, int64_t 
offset = 0);
+
+  MonthDayNanoIntervalArray(int64_t length, const std::shared_ptr<Buffer>& 
data,
+                            const std::shared_ptr<Buffer>& null_bitmap = 
NULLPTR,
+                            int64_t null_count = kUnknownNullCount, int64_t 
offset = 0);
+
+  TypeClass::MonthDayNanos GetValue(int64_t i) const;
+  TypeClass::MonthDayNanos Value(int64_t i) const { return GetValue(i); }
+
+  // For compatibility with Take kernel.
+  TypeClass::MonthDayNanos GetView(int64_t i) const { return GetValue(i); }
+
+  int32_t byte_width() const { return sizeof(TypeClass::MonthDayNanos); }
+  static_assert(sizeof(TypeClass::MonthDayNanos) == 16,
+                "MonthDayNanos should only take 16 bytes");

Review comment:
       removed.

##########
File path: cpp/src/arrow/array/array_test.cc
##########
@@ -1332,12 +1366,16 @@ TYPED_TEST(TestPrimitiveBuilder, 
TestAppendValuesLazyIter) {
 }
 
 TYPED_TEST(TestPrimitiveBuilder, TestAppendValuesIterConverted) {
-  DECL_T();
+  typedef typename TestFixture::CType T;
   // find type we can safely convert the tested values to and from
-  using conversion_type =
-      typename std::conditional<std::is_floating_point<T>::value, double,
-                                typename 
std::conditional<std::is_unsigned<T>::value,
-                                                          uint64_t, 
int64_t>::type>::type;
+  using conversion_type = typename std::conditional<
+      std::is_floating_point<T>::value, double,
+      typename std::conditional<
+          std::is_same<T, DayTimeIntervalType::DayMilliseconds>::value ||
+              std::is_same<T, MonthDayNanoIntervalType::MonthDayNanos>::value,
+          T,
+          typename std::conditional<std::is_unsigned<T>::value, uint64_t,
+                                    int64_t>::type>::type>::type;

Review comment:
       done.

##########
File path: cpp/src/arrow/ipc/json_simple.cc
##########
@@ -410,6 +410,41 @@ class DayTimeIntervalConverter final
   std::shared_ptr<DayTimeIntervalBuilder> builder_;
 };
 
+class MonthDayNanoIntervalConverter final
+    : public ConcreteConverter<MonthDayNanoIntervalConverter> {
+ public:
+  explicit MonthDayNanoIntervalConverter(const std::shared_ptr<DataType>& 
type) {
+    this->type_ = type;
+    builder_ = 
std::make_shared<MonthDayNanoIntervalBuilder>(default_memory_pool());
+  }
+
+  Status AppendValue(const rj::Value& json_obj) override {
+    if (json_obj.IsNull()) {
+      return this->AppendNull();
+    }
+    MonthDayNanoIntervalType::MonthDayNanos value;
+    if (!json_obj.IsArray()) {
+      return JSONTypeError("array", json_obj.GetType());
+    }
+    if (json_obj.Size() != 3) {
+      return Status::Invalid(
+          "month day nanos interval pair must have exactly two elements, had ",

Review comment:
       sorry, fixed.

##########
File path: cpp/src/arrow/testing/random.cc
##########
@@ -867,6 +869,7 @@ std::shared_ptr<Array> RandomArrayGenerator::ArrayOf(const 
Field& field, int64_t
       // This isn't as flexible as it could be, but the array-of-structs 
layout of this
       // type means it's not a (useful) composition of other generators
       GENERATE_INTEGRAL_CASE_VIEW(Int64Type, DayTimeIntervalType);
+      GENERATE_INTEGRAL_CASE_VIEW(Int64Type, MonthDayNanoIntervalType);

Review comment:
       should be done.

##########
File path: 
java/vector/src/main/java/org/apache/arrow/vector/IntervalMonthDayNanoVector.java
##########
@@ -0,0 +1,440 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.vector;
+
+import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;
+
+import java.time.Duration;
+import java.time.Period;
+
+import org.apache.arrow.memory.ArrowBuf;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.vector.complex.impl.IntervalMonthDayNanoReaderImpl;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.IntervalMonthDayNanoHolder;
+import org.apache.arrow.vector.holders.NullableIntervalMonthDayNanoHolder;
+import org.apache.arrow.vector.types.Types.MinorType;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.TransferPair;
+
+/**
+ * IntervalMonthDayNanoVectorimplements a fixed width vector (16 bytes) of
+ * interval (month, days and nanoseconds) values which could be null.
+ * A validity buffer (bit vector) is maintained to track which elements in the
+ * vector are null.
+ */
+public final class IntervalMonthDayNanoVector extends BaseFixedWidthVector {
+  public static final byte TYPE_WIDTH = 16;
+  private static final byte DAY_OFFSET = 4;
+  private static final byte NANOSECOND_OFFSET = 8;
+  private final FieldReader reader;
+
+
+  /**
+   * Instantiate a IntervalMonthDayNanoVector. This doesn't allocate any 
memory for
+   * the data in vector.
+   *
+   * @param name name of the vector
+   * @param allocator allocator for memory management.
+   */
+  public IntervalMonthDayNanoVector(String name, BufferAllocator allocator) {
+    this(name, FieldType.nullable(MinorType.INTERVALDAY.getType()), allocator);
+  }
+
+  /**
+   * Instantiate a IntervalMonthDayNanoVector. This doesn't allocate any 
memory for
+   * the data in vector.
+   *
+   * @param name name of the vector
+   * @param fieldType type of Field materialized by this vector
+   * @param allocator allocator for memory management.
+   */
+  public IntervalMonthDayNanoVector(String name, FieldType fieldType, 
BufferAllocator allocator) {
+    this(new Field(name, fieldType, null), allocator);
+  }
+
+  /**
+   * Instantiate a IntervalMonthDayNanoVector. This doesn't allocate any 
memory for
+   * the data in vector.
+   *
+   * @param field field materialized by this vector
+   * @param allocator allocator for memory management.
+   */
+  public IntervalMonthDayNanoVector(Field field, BufferAllocator allocator) {
+    super(field, allocator, TYPE_WIDTH);
+    reader = new 
IntervalMonthDayNanoReaderImpl(IntervalMonthDayNanoVector.this);
+  }
+
+  /**
+   * Get a reader that supports reading values from this vector.
+   *
+   * @return Field Reader for this vector
+   */
+  @Override
+  public FieldReader getReader() {
+    return reader;
+  }
+
+  /**
+   * Get minor type for this vector. The vector holds values belonging
+   * to a particular type.
+   *
+   * @return {@link org.apache.arrow.vector.types.Types.MinorType}
+   */
+  @Override
+  public MinorType getMinorType() {
+    return MinorType.INTERVALMONTHDAYNANO;
+  }
+
+
+  /*----------------------------------------------------------------*
+   |                                                                |
+   |          vector value retrieval methods                        |
+   |                                                                |
+   *----------------------------------------------------------------*/
+
+  /**
+   * Given a data buffer, get the number of months stored at a particular 
position
+   * in the vector.
+   *
+   * <p>This method should not be used externally.

Review comment:
       This unfortunately is a pattern established for integration testing 
which lives in a separate package.

##########
File path: 
java/vector/src/main/java/org/apache/arrow/vector/IntervalMonthDayNanoVector.java
##########
@@ -0,0 +1,440 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.vector;
+
+import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;
+
+import java.time.Duration;
+import java.time.Period;
+
+import org.apache.arrow.memory.ArrowBuf;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.vector.complex.impl.IntervalMonthDayNanoReaderImpl;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.IntervalMonthDayNanoHolder;
+import org.apache.arrow.vector.holders.NullableIntervalMonthDayNanoHolder;
+import org.apache.arrow.vector.types.Types.MinorType;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.TransferPair;
+
+/**
+ * IntervalMonthDayNanoVectorimplements a fixed width vector (16 bytes) of
+ * interval (month, days and nanoseconds) values which could be null.
+ * A validity buffer (bit vector) is maintained to track which elements in the
+ * vector are null.
+ */
+public final class IntervalMonthDayNanoVector extends BaseFixedWidthVector {
+  public static final byte TYPE_WIDTH = 16;
+  private static final byte DAY_OFFSET = 4;
+  private static final byte NANOSECOND_OFFSET = 8;
+  private final FieldReader reader;
+
+
+  /**
+   * Instantiate a IntervalMonthDayNanoVector. This doesn't allocate any 
memory for
+   * the data in vector.
+   *
+   * @param name name of the vector
+   * @param allocator allocator for memory management.
+   */
+  public IntervalMonthDayNanoVector(String name, BufferAllocator allocator) {
+    this(name, FieldType.nullable(MinorType.INTERVALDAY.getType()), allocator);
+  }
+
+  /**
+   * Instantiate a IntervalMonthDayNanoVector. This doesn't allocate any 
memory for
+   * the data in vector.
+   *
+   * @param name name of the vector
+   * @param fieldType type of Field materialized by this vector
+   * @param allocator allocator for memory management.
+   */
+  public IntervalMonthDayNanoVector(String name, FieldType fieldType, 
BufferAllocator allocator) {
+    this(new Field(name, fieldType, null), allocator);
+  }
+
+  /**
+   * Instantiate a IntervalMonthDayNanoVector. This doesn't allocate any 
memory for
+   * the data in vector.
+   *
+   * @param field field materialized by this vector
+   * @param allocator allocator for memory management.
+   */
+  public IntervalMonthDayNanoVector(Field field, BufferAllocator allocator) {
+    super(field, allocator, TYPE_WIDTH);
+    reader = new 
IntervalMonthDayNanoReaderImpl(IntervalMonthDayNanoVector.this);
+  }
+
+  /**
+   * Get a reader that supports reading values from this vector.
+   *
+   * @return Field Reader for this vector
+   */
+  @Override
+  public FieldReader getReader() {
+    return reader;
+  }
+
+  /**
+   * Get minor type for this vector. The vector holds values belonging
+   * to a particular type.
+   *
+   * @return {@link org.apache.arrow.vector.types.Types.MinorType}
+   */
+  @Override
+  public MinorType getMinorType() {
+    return MinorType.INTERVALMONTHDAYNANO;
+  }
+
+
+  /*----------------------------------------------------------------*
+   |                                                                |
+   |          vector value retrieval methods                        |
+   |                                                                |
+   *----------------------------------------------------------------*/
+
+  /**
+   * Given a data buffer, get the number of months stored at a particular 
position
+   * in the vector.
+   *
+   * <p>This method should not be used externally.
+   *
+   * @param buffer data buffer
+   * @param index  position of the element.
+   * @return day value stored at the index.
+   */
+  public static int getMonths(final ArrowBuf buffer, final int index) {
+    return buffer.getInt((long) index * TYPE_WIDTH);
+  }
+
+
+  /**
+   * Given a data buffer, get the number of days stored at a particular 
position
+   * in the vector.
+   *
+   * <p>This method should not be used externally.
+   *
+   * @param buffer data buffer
+   * @param index  position of the element.
+   * @return day value stored at the index.
+   */
+  public static int getDays(final ArrowBuf buffer, final int index) {
+    return buffer.getInt((long) index * TYPE_WIDTH + DAY_OFFSET);
+  }
+
+  /**
+   * Given a data buffer, get the get the number of nanoseconds stored at a 
particular position
+   * in the vector.
+   *
+   * <p>This method should not be used externally.
+   *
+   * @param buffer data buffer
+   * @param index  position of the element.
+   * @return nanoseconds value stored at the index.
+   */
+  public static long getNanoseconds(final ArrowBuf buffer, final int index) {
+    return buffer.getLong((long) index * TYPE_WIDTH + NANOSECOND_OFFSET);
+  }
+
+  /**
+   * Get the element at the given index from the vector.
+   *
+   * @param index   position of element
+   * @return element at given index
+   */
+  public ArrowBuf get(int index) throws IllegalStateException {
+    if (NULL_CHECKING_ENABLED && isSet(index) == 0) {
+      return null;
+    }
+    return valueBuffer.slice((long) index * TYPE_WIDTH, TYPE_WIDTH);
+  }
+
+  /**
+   * Get the element at the given index from the vector and
+   * sets the state in holder. If element at given index
+   * is null, holder.isSet will be zero.
+   *
+   * @param index   position of element
+   */
+  public void get(int index, NullableIntervalMonthDayNanoHolder holder) {
+    if (isSet(index) == 0) {
+      holder.isSet = 0;
+      return;
+    }
+    final long startIndex = (long) index * TYPE_WIDTH;
+    holder.isSet = 1;
+    holder.months = valueBuffer.getInt(startIndex);
+    holder.days = valueBuffer.getInt(startIndex + DAY_OFFSET);
+    holder.nanoseconds = valueBuffer.getLong(startIndex + NANOSECOND_OFFSET);
+  }
+
+  /**
+   * Same as {@link #get(int)}.
+   *
+   * @param index   position of element
+   * @return element at given index
+   */
+  public PeriodDuration getObject(int index) {
+    if (isSet(index) == 0) {
+      return null;
+    } else {
+      final long startIndex = (long) index * TYPE_WIDTH;
+      final int months = valueBuffer.getInt(startIndex);
+      final int days = valueBuffer.getInt(startIndex + DAY_OFFSET);
+      final long nanoseconds = valueBuffer.getLong(startIndex + 
NANOSECOND_OFFSET);
+
+      return new PeriodDuration(Period.ofMonths(months).plusDays(days),
+            Duration.ofNanos(nanoseconds));
+    }
+  }
+
+  /**
+   * Get the Interval value at a given index as a {@link StringBuilder} object.
+   *
+   * @param index position of the element
+   * @return String Builder object with Interval value as
+   */
+  public StringBuilder getAsStringBuilder(int index) {
+    if (isSet(index) == 0) {
+      return null;
+    } else {
+      return getAsStringBuilderHelper(index);
+    }
+  }
+
+  private StringBuilder getAsStringBuilderHelper(int index) {
+    final long startIndex = (long) index * TYPE_WIDTH;

Review comment:
       nice catch.

##########
File path: cpp/src/arrow/testing/random.cc
##########
@@ -867,6 +869,7 @@ std::shared_ptr<Array> RandomArrayGenerator::ArrayOf(const 
Field& field, int64_t
       // This isn't as flexible as it could be, but the array-of-structs 
layout of this
       // type means it's not a (useful) composition of other generators
       GENERATE_INTEGRAL_CASE_VIEW(Int64Type, DayTimeIntervalType);
+      GENERATE_INTEGRAL_CASE_VIEW(Int64Type, MonthDayNanoIntervalType);

Review comment:
       sorry still need to add a test.

##########
File path: java/vector/src/main/java/org/apache/arrow/vector/PeriodDuration.java
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.vector;
+
+import java.time.Duration;
+import java.time.Period;
+
+/**
+ * Combination of Period and Duration for representing this interval type
+ * as a POJO.
+ */
+public class PeriodDuration {
+  private final Period period;
+  private final Duration duration;
+
+  public PeriodDuration(Period period, Duration duration) {
+    this.period = period;
+    this.duration = duration;
+  }
+
+  public Period getPeriod() {
+    return period;
+  }
+
+  public Duration getDuration() {
+    return duration;
+  }
+
+  @Override
+  public String toString() {
+    return period.toString() + " " + duration.toString();
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (!(o instanceof PeriodDuration)) {
+      return false;
+    }
+    PeriodDuration other = (PeriodDuration) o;
+    return this.period.equals(other.period) && 
this.duration.equals(other.duration);

Review comment:
       no .added check not null.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to