Wanted to give this a bump to see if the timing is better now for the folks at Dremio to take a look at the impact switching to Java 8 Time? PR at https://github.com/apache/arrow/pull/2966
Thanks, Bryan On Tue, Nov 20, 2018 at 10:41 PM Praveen Kumar <prav...@dremio.com> wrote: > Jacques, > > Shyam is trying to build dremio against the patch to assess impact. Will > keep the list posted as we progress. > > Thx. > > On Fri, Nov 16, 2018 at 9:46 AM Jacques Nadeau <jacq...@apache.org> wrote: > > > I'm worried about the change from Millis to Nanos. It seems not so good. > I > > think we (dremio) need to better understand impact. Praveen, maybe you > > could take a look at impact? > > > > On Wed, Nov 14, 2018, 4:27 PM Bryan Cutler <cutl...@gmail.com wrote: > > > > > Hi all, > > > > > > I'm picking this back up again and have WIP pr at > > > https://github.com/apache/arrow/pull/2966. Please take a look at the > new > > > APIs and see if they impact you downstream. In addition to the API > > changes > > > mentioned before by Li, there is also > > > > > > (4) IntervalDayVector now uses java.time.Duration, while > > IntervalYearVector > > > uses java.time.Period. I think Joda Period was basically a combination > of > > > these two. > > > > > > Thanks, > > > Bryan > > > > > > On Thu, Jun 28, 2018 at 2:31 PM Li Jin <ice.xell...@gmail.com> wrote: > > > > > > > I did a grep of "joda" in the java vector codebase. The current > > reference > > > > to joda includes: > > > > > > > > (1) auto-generated reader class under arrow/vector/complex/impl and > > > > arrow/vector/complex/reader > > > > > > > > These class currently have API that returns joda classes, e.g.: > > > > > > > > NullableTimestampMicroHolderReaderImpl: > > > > > > > > // read friendly type > > > > @Override > > > > public LocalDateTime readLocalDateTime() { > > > > if (!isSet()) { > > > > return null; > > > > } > > > > > > > > LocalDateTime value = new LocalDateTime(this.holder.value); > > > > return value; > > > > } > > > > > > > > UnionReader: > > > > > > > > @Override > > > > public LocalDateTime readLocalDateTime() { > > > > return getReaderForIndex(idx()).readLocalDateTime(); > > > > } > > > > > > > > > > > > (2) Timestamp vector classes: > > > > These classes have API that returns joda classes, e.g.: > > > > > > > > TimestampMicroVector: > > > > > > > > /** > > > > * Same as {@link #get(int)}. > > > > * > > > > * @param index position of element > > > > * @return element at given index > > > > */ > > > > public LocalDateTime getObject(int index) { > > > > if (isSet(index) == 0) { > > > > return null; > > > > } else { > > > > /* value is truncated when converting microseconds to > > > > milliseconds in order to use DateTime type */ > > > > final long micros = valueBuffer.getLong(index * TYPE_WIDTH); > > > > final long millis = > > > > java.util.concurrent.TimeUnit.MICROSECONDS.toMillis(micros); > > > > final org.joda.time.LocalDateTime localDateTime = new > > > > org.joda.time.LocalDateTime(millis, > > > > org.joda.time.DateTimeZone.UTC); > > > > return localDateTime; > > > > } > > > > } > > > > > > > > > > > > (3) arrow/vector/util/DateUtility.java has some functions that uses > > joda > > > > time > > > > > > > > > > > > On Thu, Jun 28, 2018 at 2:19 PM, Jacques Nadeau <jacq...@apache.org> > > > > wrote: > > > > > > > > > Can we get a proposed listing of apis that would change? Definitely > > > will > > > > be > > > > > effort for people like us to rewrite all code to use different > items. > > > > > > > > > > On Wed, Jun 27, 2018 at 10:54 AM, Li Jin <ice.xell...@gmail.com> > > > wrote: > > > > > > > > > > > Hi, > > > > > > > > > > > > There has been a recent pr 2171 for ARROW-2015 to replace Joda > time > > > > with > > > > > > Java8 time. > > > > > > > > > > > > I think this change is good as we move toward newer Java version > > and > > > > > wonder > > > > > > if we should include this in the 0.10 release. > > > > > > > > > > > > The biggest concern is that this is a breaking change and could > > > impact > > > > > > downstream projects like Dremio. What do people think? > > > > > > > > > > > > Li > > > > > > > > > > > > > > > > > > > > >
