You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2020/12/09 15:09:26 UTC
[GitHub] [iceberg] marton-bod opened a new pull request #1897: Add support for timestamp with local zone in Hive3
marton-bod opened a new pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r540248379
##########
File path: hive3/src/test/java/org/apache/iceberg/mr/hive/serde/objectinspector/TestIcebergTimestampWithZoneObjectInspectorHive3.java
##########
@@ -0,0 +1,69 @@
+/*
+ * 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.iceberg.mr.hive.serde.objectinspector;
+
+import java.time.LocalDateTime;
+import java.time.OffsetDateTime;
+import java.time.ZoneOffset;
+import org.apache.hadoop.hive.common.type.TimestampTZ;
+import org.apache.hadoop.hive.serde2.io.TimestampLocalTZWritable;
+import org.apache.hadoop.hive.serde2.objectinspector.ObjectInspector;
+import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector;
+import org.apache.hadoop.hive.serde2.objectinspector.primitive.TimestampLocalTZObjectInspector;
+import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestIcebergTimestampWithZoneObjectInspectorHive3 {
+
+ @Test
+ public void testIcebergTimestampLocalTZObjectInspector() {
+ TimestampLocalTZObjectInspector oi = IcebergTimestampWithZoneObjectInspectorHive3.get();
+
+ Assert.assertEquals(ObjectInspector.Category.PRIMITIVE, oi.getCategory());
+ Assert.assertEquals(PrimitiveObjectInspector.PrimitiveCategory.TIMESTAMPLOCALTZ, oi.getPrimitiveCategory());
+
+ Assert.assertEquals(TypeInfoFactory.timestampLocalTZTypeInfo, oi.getTypeInfo());
+ Assert.assertEquals(TypeInfoFactory.timestampLocalTZTypeInfo.getTypeName(), oi.getTypeName());
+
+ Assert.assertEquals(TimestampTZ.class, oi.getJavaPrimitiveClass());
+ Assert.assertEquals(TimestampLocalTZWritable.class, oi.getPrimitiveWritableClass());
+
+ Assert.assertNull(oi.copyObject(null));
+ Assert.assertNull(oi.getPrimitiveJavaObject(null));
+ Assert.assertNull(oi.getPrimitiveWritableObject(null));
+
+ long epochSeconds = 1601471970L;
Review comment:
Refactored this to use LocalDateTime instead
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r541544634
##########
File path: hive-metastore/src/main/java/org/apache/iceberg/hive/HiveSchemaConverter.java
##########
@@ -99,6 +99,10 @@ Type convertType(TypeInfo typeInfo) {
case INTERVAL_YEAR_MONTH:
case INTERVAL_DAY_TIME:
default:
+ // special case for Timestamp with Local TZ which is only available in Hive3
+ if ("TIMESTAMPLOCALTZ".equals(((PrimitiveTypeInfo) typeInfo).getPrimitiveCategory().name())) {
Review comment:
I think it should always be all uppercase, but I've changed it just to be safe.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r541129980
##########
File path: hive-metastore/src/main/java/org/apache/iceberg/hive/HiveSchemaConverter.java
##########
@@ -99,6 +99,10 @@ Type convertType(TypeInfo typeInfo) {
case INTERVAL_YEAR_MONTH:
case INTERVAL_DAY_TIME:
default:
+ // special case for Timestamp with Local TZ which is only available in Hive3
+ if ("TIMESTAMPLOCALTZ".equals(((PrimitiveTypeInfo) typeInfo).getPrimitiveCategory().name())) {
Review comment:
`equalsIgnoreCase`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r540006994
##########
File path: mr/src/main/java/org/apache/iceberg/mr/hive/serde/objectinspector/IcebergTimestampObjectInspector.java
##########
@@ -80,22 +57,16 @@ public TimestampWritable getPrimitiveWritableObject(Object o) {
@Override
public Object copyObject(Object o) {
- if (o == null) {
Review comment:
Why did we remove this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#issuecomment-742414487
Do we need to change `HiveSchemaConverter`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r540003682
##########
File path: hive3/src/main/java/org/apache/iceberg/mr/hive/serde/objectinspector/IcebergTimestampObjectInspectorHive3.java
##########
@@ -20,50 +20,32 @@
package org.apache.iceberg.mr.hive.serde.objectinspector;
import java.time.LocalDateTime;
-import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import org.apache.hadoop.hive.common.type.Timestamp;
import org.apache.hadoop.hive.serde2.io.TimestampWritableV2;
import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveJavaObjectInspector;
import org.apache.hadoop.hive.serde2.objectinspector.primitive.TimestampObjectInspector;
import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
-public abstract class IcebergTimestampObjectInspectorHive3 extends AbstractPrimitiveJavaObjectInspector
+public class IcebergTimestampObjectInspectorHive3 extends AbstractPrimitiveJavaObjectInspector
Review comment:
You might want to rebase to master. We need to implement WriteObjectInspector.
Also we need to test for them as well
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r541309064
##########
File path: hive3/src/test/java/org/apache/iceberg/mr/hive/serde/objectinspector/TestIcebergTimestampObjectInspectorHive3.java
##########
@@ -51,6 +48,7 @@ public void testIcebergTimestampObjectInspector() {
Assert.assertNull(oi.copyObject(null));
Assert.assertNull(oi.getPrimitiveJavaObject(null));
Assert.assertNull(oi.getPrimitiveWritableObject(null));
+ Assert.assertNull(oi.convert(null));
long epochMilli = 1601471970000L;
LocalDateTime local = LocalDateTime.ofInstant(Instant.ofEpochMilli(epochMilli), ZoneId.of("UTC"));
Review comment:
Yes!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
marton-bod commented on pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#issuecomment-742024777
@pvary can you please review when you have the chance?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r540994252
##########
File path: hive3/src/main/java/org/apache/iceberg/mr/hive/serde/objectinspector/IcebergTimestampObjectInspectorHive3.java
##########
@@ -20,50 +20,38 @@
package org.apache.iceberg.mr.hive.serde.objectinspector;
import java.time.LocalDateTime;
-import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import org.apache.hadoop.hive.common.type.Timestamp;
import org.apache.hadoop.hive.serde2.io.TimestampWritableV2;
import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveJavaObjectInspector;
import org.apache.hadoop.hive.serde2.objectinspector.primitive.TimestampObjectInspector;
import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
-public abstract class IcebergTimestampObjectInspectorHive3 extends AbstractPrimitiveJavaObjectInspector
- implements TimestampObjectInspector {
+public class IcebergTimestampObjectInspectorHive3 extends AbstractPrimitiveJavaObjectInspector
+ implements TimestampObjectInspector, WriteObjectInspector {
- private static final IcebergTimestampObjectInspectorHive3 INSTANCE_WITH_ZONE =
- new IcebergTimestampObjectInspectorHive3() {
- @Override
- LocalDateTime toLocalDateTime(Object o) {
- return ((OffsetDateTime) o).toLocalDateTime();
- }
- };
+ private static final IcebergTimestampObjectInspectorHive3 INSTANCE = new IcebergTimestampObjectInspectorHive3();
- private static final IcebergTimestampObjectInspectorHive3 INSTANCE_WITHOUT_ZONE =
- new IcebergTimestampObjectInspectorHive3() {
- @Override
- LocalDateTime toLocalDateTime(Object o) {
- return (LocalDateTime) o;
- }
- };
-
- public static IcebergTimestampObjectInspectorHive3 get(boolean adjustToUTC) {
- return adjustToUTC ? INSTANCE_WITH_ZONE : INSTANCE_WITHOUT_ZONE;
+ public static IcebergTimestampObjectInspectorHive3 get() {
+ return INSTANCE;
}
private IcebergTimestampObjectInspectorHive3() {
super(TypeInfoFactory.timestampTypeInfo);
}
-
- abstract LocalDateTime toLocalDateTime(Object object);
+ @Override
+ public LocalDateTime convert(Object o) {
+ return o == null ? null : LocalDateTime.ofEpochSecond(
+ ((TimestampWritableV2) o).getTimestamp().toEpochSecond(), 0, ZoneOffset.UTC);
Review comment:
Do we lose nanos here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r540024696
##########
File path: hive3/src/test/java/org/apache/iceberg/mr/hive/serde/objectinspector/TestIcebergTimestampWithZoneObjectInspectorHive3.java
##########
@@ -0,0 +1,69 @@
+/*
+ * 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.iceberg.mr.hive.serde.objectinspector;
+
+import java.time.LocalDateTime;
+import java.time.OffsetDateTime;
+import java.time.ZoneOffset;
+import org.apache.hadoop.hive.common.type.TimestampTZ;
+import org.apache.hadoop.hive.serde2.io.TimestampLocalTZWritable;
+import org.apache.hadoop.hive.serde2.objectinspector.ObjectInspector;
+import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector;
+import org.apache.hadoop.hive.serde2.objectinspector.primitive.TimestampLocalTZObjectInspector;
+import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestIcebergTimestampWithZoneObjectInspectorHive3 {
+
+ @Test
+ public void testIcebergTimestampLocalTZObjectInspector() {
+ TimestampLocalTZObjectInspector oi = IcebergTimestampWithZoneObjectInspectorHive3.get();
+
+ Assert.assertEquals(ObjectInspector.Category.PRIMITIVE, oi.getCategory());
+ Assert.assertEquals(PrimitiveObjectInspector.PrimitiveCategory.TIMESTAMPLOCALTZ, oi.getPrimitiveCategory());
+
+ Assert.assertEquals(TypeInfoFactory.timestampLocalTZTypeInfo, oi.getTypeInfo());
+ Assert.assertEquals(TypeInfoFactory.timestampLocalTZTypeInfo.getTypeName(), oi.getTypeName());
+
+ Assert.assertEquals(TimestampTZ.class, oi.getJavaPrimitiveClass());
+ Assert.assertEquals(TimestampLocalTZWritable.class, oi.getPrimitiveWritableClass());
+
+ Assert.assertNull(oi.copyObject(null));
+ Assert.assertNull(oi.getPrimitiveJavaObject(null));
+ Assert.assertNull(oi.getPrimitiveWritableObject(null));
+
+ long epochSeconds = 1601471970L;
+ OffsetDateTime offsetDateTime = OffsetDateTime.of(
Review comment:
Yes, that's a good idea.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r541007036
##########
File path: hive3/src/main/java/org/apache/iceberg/mr/hive/serde/objectinspector/IcebergTimestampObjectInspectorHive3.java
##########
@@ -20,50 +20,38 @@
package org.apache.iceberg.mr.hive.serde.objectinspector;
import java.time.LocalDateTime;
-import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import org.apache.hadoop.hive.common.type.Timestamp;
import org.apache.hadoop.hive.serde2.io.TimestampWritableV2;
import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveJavaObjectInspector;
import org.apache.hadoop.hive.serde2.objectinspector.primitive.TimestampObjectInspector;
import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
-public abstract class IcebergTimestampObjectInspectorHive3 extends AbstractPrimitiveJavaObjectInspector
- implements TimestampObjectInspector {
+public class IcebergTimestampObjectInspectorHive3 extends AbstractPrimitiveJavaObjectInspector
+ implements TimestampObjectInspector, WriteObjectInspector {
- private static final IcebergTimestampObjectInspectorHive3 INSTANCE_WITH_ZONE =
- new IcebergTimestampObjectInspectorHive3() {
- @Override
- LocalDateTime toLocalDateTime(Object o) {
- return ((OffsetDateTime) o).toLocalDateTime();
- }
- };
+ private static final IcebergTimestampObjectInspectorHive3 INSTANCE = new IcebergTimestampObjectInspectorHive3();
- private static final IcebergTimestampObjectInspectorHive3 INSTANCE_WITHOUT_ZONE =
- new IcebergTimestampObjectInspectorHive3() {
- @Override
- LocalDateTime toLocalDateTime(Object o) {
- return (LocalDateTime) o;
- }
- };
-
- public static IcebergTimestampObjectInspectorHive3 get(boolean adjustToUTC) {
- return adjustToUTC ? INSTANCE_WITH_ZONE : INSTANCE_WITHOUT_ZONE;
+ public static IcebergTimestampObjectInspectorHive3 get() {
+ return INSTANCE;
}
private IcebergTimestampObjectInspectorHive3() {
super(TypeInfoFactory.timestampTypeInfo);
}
-
- abstract LocalDateTime toLocalDateTime(Object object);
+ @Override
+ public LocalDateTime convert(Object o) {
+ return o == null ? null : LocalDateTime.ofEpochSecond(
+ ((TimestampWritableV2) o).getTimestamp().toEpochSecond(), 0, ZoneOffset.UTC);
Review comment:
we did, but we shouldn't :) fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r540005579
##########
File path: hive3/src/test/java/org/apache/iceberg/mr/hive/serde/objectinspector/TestIcebergTimestampWithZoneObjectInspectorHive3.java
##########
@@ -0,0 +1,69 @@
+/*
+ * 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.iceberg.mr.hive.serde.objectinspector;
+
+import java.time.LocalDateTime;
+import java.time.OffsetDateTime;
+import java.time.ZoneOffset;
+import org.apache.hadoop.hive.common.type.TimestampTZ;
+import org.apache.hadoop.hive.serde2.io.TimestampLocalTZWritable;
+import org.apache.hadoop.hive.serde2.objectinspector.ObjectInspector;
+import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector;
+import org.apache.hadoop.hive.serde2.objectinspector.primitive.TimestampLocalTZObjectInspector;
+import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestIcebergTimestampWithZoneObjectInspectorHive3 {
+
+ @Test
+ public void testIcebergTimestampLocalTZObjectInspector() {
+ TimestampLocalTZObjectInspector oi = IcebergTimestampWithZoneObjectInspectorHive3.get();
+
+ Assert.assertEquals(ObjectInspector.Category.PRIMITIVE, oi.getCategory());
+ Assert.assertEquals(PrimitiveObjectInspector.PrimitiveCategory.TIMESTAMPLOCALTZ, oi.getPrimitiveCategory());
+
+ Assert.assertEquals(TypeInfoFactory.timestampLocalTZTypeInfo, oi.getTypeInfo());
+ Assert.assertEquals(TypeInfoFactory.timestampLocalTZTypeInfo.getTypeName(), oi.getTypeName());
+
+ Assert.assertEquals(TimestampTZ.class, oi.getJavaPrimitiveClass());
+ Assert.assertEquals(TimestampLocalTZWritable.class, oi.getPrimitiveWritableClass());
+
+ Assert.assertNull(oi.copyObject(null));
+ Assert.assertNull(oi.getPrimitiveJavaObject(null));
+ Assert.assertNull(oi.getPrimitiveWritableObject(null));
+
+ long epochSeconds = 1601471970L;
+ OffsetDateTime offsetDateTime = OffsetDateTime.of(
Review comment:
Could we test that the same instant with different offset will result in the same TimestampTZ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
marton-bod commented on pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#issuecomment-742584273
> Do we need to change `HiveSchemaConverter`?
Yes, good point. I'll modify the `HiveSchemaConverter` as well as the `HiveSchemaUtil` classes. Adjusting their unit tests are tricky, because the hive-metastore unit tests are run using Hive2 classes. I'll need to think how to go around that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r540006136
##########
File path: hive3/src/test/java/org/apache/iceberg/mr/hive/serde/objectinspector/TestIcebergTimestampWithZoneObjectInspectorHive3.java
##########
@@ -0,0 +1,69 @@
+/*
+ * 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.iceberg.mr.hive.serde.objectinspector;
+
+import java.time.LocalDateTime;
+import java.time.OffsetDateTime;
+import java.time.ZoneOffset;
+import org.apache.hadoop.hive.common.type.TimestampTZ;
+import org.apache.hadoop.hive.serde2.io.TimestampLocalTZWritable;
+import org.apache.hadoop.hive.serde2.objectinspector.ObjectInspector;
+import org.apache.hadoop.hive.serde2.objectinspector.PrimitiveObjectInspector;
+import org.apache.hadoop.hive.serde2.objectinspector.primitive.TimestampLocalTZObjectInspector;
+import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestIcebergTimestampWithZoneObjectInspectorHive3 {
+
+ @Test
+ public void testIcebergTimestampLocalTZObjectInspector() {
+ TimestampLocalTZObjectInspector oi = IcebergTimestampWithZoneObjectInspectorHive3.get();
+
+ Assert.assertEquals(ObjectInspector.Category.PRIMITIVE, oi.getCategory());
+ Assert.assertEquals(PrimitiveObjectInspector.PrimitiveCategory.TIMESTAMPLOCALTZ, oi.getPrimitiveCategory());
+
+ Assert.assertEquals(TypeInfoFactory.timestampLocalTZTypeInfo, oi.getTypeInfo());
+ Assert.assertEquals(TypeInfoFactory.timestampLocalTZTypeInfo.getTypeName(), oi.getTypeName());
+
+ Assert.assertEquals(TimestampTZ.class, oi.getJavaPrimitiveClass());
+ Assert.assertEquals(TimestampLocalTZWritable.class, oi.getPrimitiveWritableClass());
+
+ Assert.assertNull(oi.copyObject(null));
+ Assert.assertNull(oi.getPrimitiveJavaObject(null));
+ Assert.assertNull(oi.getPrimitiveWritableObject(null));
+
+ long epochSeconds = 1601471970L;
Review comment:
How can we be sure that this epoch seconds really result in the expected TZ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#issuecomment-743343878
@pvary, please ping me when this is ready to merge.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r541287234
##########
File path: hive3/src/test/java/org/apache/iceberg/mr/hive/TestHiveSchemaUtilHive3.java
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.iceberg.mr.hive;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.hadoop.hive.metastore.api.FieldSchema;
+import org.apache.hadoop.hive.serde.serdeConstants;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.hive.TestHiveSchemaUtil;
+import org.apache.iceberg.types.Types;
+
+import static org.apache.iceberg.types.Types.NestedField.optional;
+
+public class TestHiveSchemaUtilHive3 extends TestHiveSchemaUtil {
+
+ @Override
+ protected List<FieldSchema> getSupportedFieldSchemas() {
Review comment:
Is it possible to use super and add only specific fields?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r540249853
##########
File path: mr/src/main/java/org/apache/iceberg/mr/hive/serde/objectinspector/IcebergTimestampObjectInspector.java
##########
@@ -80,22 +57,16 @@ public TimestampWritable getPrimitiveWritableObject(Object o) {
@Override
public Object copyObject(Object o) {
- if (o == null) {
Review comment:
this null check is handled in the `else` branch
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r541544344
##########
File path: hive3/src/test/java/org/apache/iceberg/mr/hive/serde/objectinspector/TestIcebergTimestampObjectInspectorHive3.java
##########
@@ -51,6 +48,7 @@ public void testIcebergTimestampObjectInspector() {
Assert.assertNull(oi.copyObject(null));
Assert.assertNull(oi.getPrimitiveJavaObject(null));
Assert.assertNull(oi.getPrimitiveWritableObject(null));
+ Assert.assertNull(oi.convert(null));
long epochMilli = 1601471970000L;
LocalDateTime local = LocalDateTime.ofInstant(Instant.ofEpochMilli(epochMilli), ZoneId.of("UTC"));
Review comment:
Done!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r541290219
##########
File path: hive3/src/test/java/org/apache/iceberg/mr/hive/serde/objectinspector/TestIcebergTimestampObjectInspectorHive3.java
##########
@@ -51,6 +48,7 @@ public void testIcebergTimestampObjectInspector() {
Assert.assertNull(oi.copyObject(null));
Assert.assertNull(oi.getPrimitiveJavaObject(null));
Assert.assertNull(oi.getPrimitiveWritableObject(null));
+ Assert.assertNull(oi.convert(null));
long epochMilli = 1601471970000L;
LocalDateTime local = LocalDateTime.ofInstant(Instant.ofEpochMilli(epochMilli), ZoneId.of("UTC"));
Review comment:
We do not check nanos. Do we want to do it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r540024234
##########
File path: hive3/src/main/java/org/apache/iceberg/mr/hive/serde/objectinspector/IcebergTimestampObjectInspectorHive3.java
##########
@@ -20,50 +20,32 @@
package org.apache.iceberg.mr.hive.serde.objectinspector;
import java.time.LocalDateTime;
-import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import org.apache.hadoop.hive.common.type.Timestamp;
import org.apache.hadoop.hive.serde2.io.TimestampWritableV2;
import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveJavaObjectInspector;
import org.apache.hadoop.hive.serde2.objectinspector.primitive.TimestampObjectInspector;
import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
-public abstract class IcebergTimestampObjectInspectorHive3 extends AbstractPrimitiveJavaObjectInspector
+public class IcebergTimestampObjectInspectorHive3 extends AbstractPrimitiveJavaObjectInspector
Review comment:
I've done a merge with master, and this class `IcebergTimestampObjectInspectorHive3` doesn't seem to implement `WriteObjectInspector` yet even on master. I can add a similar convert method that we have for `IcebergTimestampObjectInspector`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on a change in pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#discussion_r541544378
##########
File path: hive3/src/test/java/org/apache/iceberg/mr/hive/TestHiveSchemaUtilHive3.java
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.iceberg.mr.hive;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.hadoop.hive.metastore.api.FieldSchema;
+import org.apache.hadoop.hive.serde.serdeConstants;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.hive.TestHiveSchemaUtil;
+import org.apache.iceberg.types.Types;
+
+import static org.apache.iceberg.types.Types.NestedField.optional;
+
+public class TestHiveSchemaUtilHive3 extends TestHiveSchemaUtil {
+
+ @Override
+ protected List<FieldSchema> getSupportedFieldSchemas() {
Review comment:
yes, changed it to use super
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897#issuecomment-743876007
Merged. Thanks @marton-bod, and thanks for reviewing, @pvary!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #1897: Add support for timestamp with local zone in Hive3
Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #1897:
URL: https://github.com/apache/iceberg/pull/1897
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org