pgaref commented on a change in pull request #631:
URL: https://github.com/apache/orc/pull/631#discussion_r559488673
##########
File path: java/tools/src/java/org/apache/orc/tools/convert/CsvReader.java
##########
@@ -19,27 +19,19 @@
import com.opencsv.CSVReader;
import org.apache.hadoop.fs.FSDataInputStream;
-import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.StructColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.*;
import org.apache.hadoop.hive.serde2.io.HiveDecimalWritable;
import org.apache.hadoop.io.IntWritable;
import org.apache.orc.RecordReader;
import org.apache.orc.TypeDescription;
-import org.threeten.bp.LocalDateTime;
-import org.threeten.bp.ZoneId;
-import org.threeten.bp.ZonedDateTime;
-import org.threeten.bp.format.DateTimeFormatter;
-import org.threeten.bp.temporal.TemporalAccessor;
+
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.sql.Timestamp;
+import java.time.*;
Review comment:
Please use fully qualifies names for imports
##########
File path: java/tools/src/java/org/apache/orc/tools/convert/CsvReader.java
##########
@@ -252,16 +268,20 @@ public void convert(String[] values, ColumnVector column,
int row) {
TimestampColumnVector vector = (TimestampColumnVector) column;
TemporalAccessor temporalAccessor =
dateTimeFormatter.parseBest(values[offset],
- ZonedDateTime.FROM, LocalDateTime.FROM);
+ ZonedDateTime::from, OffsetDateTime::from,
LocalDateTime::from);
if (temporalAccessor instanceof ZonedDateTime) {
ZonedDateTime zonedDateTime = ((ZonedDateTime) temporalAccessor);
- Timestamp timestamp = new Timestamp(zonedDateTime.toEpochSecond() *
1000L);
- timestamp.setNanos(zonedDateTime.getNano());
+ Timestamp timestamp = Timestamp.from(zonedDateTime.toInstant());
vector.set(row, timestamp);
- } else if (temporalAccessor instanceof LocalDateTime) {
+ }
+ else if (temporalAccessor instanceof OffsetDateTime) {
+ OffsetDateTime offsetDateTime = (OffsetDateTime) temporalAccessor;
+ Timestamp timestamp = Timestamp.from(offsetDateTime.toInstant());
+ vector.set(row, timestamp);
+ }
+ else if (temporalAccessor instanceof LocalDateTime) {
Review comment:
nit: style
##########
File path: java/tools/src/java/org/apache/orc/tools/convert/CsvReader.java
##########
@@ -252,16 +268,20 @@ public void convert(String[] values, ColumnVector column,
int row) {
TimestampColumnVector vector = (TimestampColumnVector) column;
TemporalAccessor temporalAccessor =
dateTimeFormatter.parseBest(values[offset],
- ZonedDateTime.FROM, LocalDateTime.FROM);
+ ZonedDateTime::from, OffsetDateTime::from,
LocalDateTime::from);
if (temporalAccessor instanceof ZonedDateTime) {
ZonedDateTime zonedDateTime = ((ZonedDateTime) temporalAccessor);
- Timestamp timestamp = new Timestamp(zonedDateTime.toEpochSecond() *
1000L);
- timestamp.setNanos(zonedDateTime.getNano());
+ Timestamp timestamp = Timestamp.from(zonedDateTime.toInstant());
vector.set(row, timestamp);
- } else if (temporalAccessor instanceof LocalDateTime) {
+ }
+ else if (temporalAccessor instanceof OffsetDateTime) {
Review comment:
follow existing style:
} else if
##########
File path: java/tools/src/java/org/apache/orc/tools/convert/JsonReader.java
##########
@@ -24,28 +24,19 @@
import com.google.gson.JsonStreamParser;
import org.apache.hadoop.fs.FSDataInputStream;
import org.apache.hadoop.hive.common.type.HiveDecimal;
-import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.ListColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.MapColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.StructColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.*;
Review comment:
nit: fully qualified names for imports
##########
File path: java/tools/src/java/org/apache/orc/tools/convert/JsonReader.java
##########
@@ -24,28 +24,19 @@
import com.google.gson.JsonStreamParser;
import org.apache.hadoop.fs.FSDataInputStream;
import org.apache.hadoop.hive.common.type.HiveDecimal;
-import org.apache.hadoop.hive.ql.exec.vector.BytesColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.ColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.DecimalColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.ListColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.LongColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.MapColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.StructColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.TimestampColumnVector;
-import org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch;
+import org.apache.hadoop.hive.ql.exec.vector.*;
import org.apache.orc.RecordReader;
import org.apache.orc.TypeDescription;
-import org.threeten.bp.LocalDateTime;
-import org.threeten.bp.ZonedDateTime;
-import org.threeten.bp.ZoneId;
-import org.threeten.bp.format.DateTimeFormatter;
-import org.threeten.bp.temporal.TemporalAccessor;
+
+
import java.io.IOException;
import java.io.Reader;
import java.nio.charset.StandardCharsets;
import java.sql.Timestamp;
+import java.time.*;
Review comment:
same as above
##########
File path: java/tools/src/java/org/apache/orc/tools/convert/JsonReader.java
##########
@@ -144,16 +156,20 @@ public void convert(JsonElement value, ColumnVector vect,
int row) {
} else {
TimestampColumnVector vector = (TimestampColumnVector) vect;
TemporalAccessor temporalAccessor =
dateTimeFormatter.parseBest(value.getAsString(),
- ZonedDateTime.FROM, LocalDateTime.FROM);
+ ZonedDateTime::from, OffsetDateTime::from, LocalDateTime::from);
if (temporalAccessor instanceof ZonedDateTime) {
ZonedDateTime zonedDateTime = ((ZonedDateTime) temporalAccessor);
- Timestamp timestamp = new Timestamp(zonedDateTime.toEpochSecond() *
1000L);
- timestamp.setNanos(zonedDateTime.getNano());
+ Timestamp timestamp = Timestamp.from(zonedDateTime.toInstant());
+ vector.set(row, timestamp);
+ }
+ else if (temporalAccessor instanceof OffsetDateTime) {
Review comment:
not: follow existing style
##########
File path: java/tools/src/test/org/apache/orc/tools/convert/TestJsonReader.java
##########
@@ -72,4 +75,25 @@ public void testTimestampOffByOne() throws Exception {
assertEquals("1969-12-31 23:59:58.9999",
cv.asScratchTimestamp(5).toString());
}
+ @Test
+ public void testDateTypeSupport() throws IOException {
+ LocalDate date1 = LocalDate.of(2021, 1, 18);
+ LocalDate date2 = LocalDate.now();
+ String inputString = "{\"dt\": \"" + date1.toString() + "\"}\n" +
+ "{\"dt\": \"" + date2.toString() + "\"}\n";
+
+
+ StringReader input = new StringReader(inputString);
+
+ TypeDescription schema = TypeDescription.fromString("struct<dt:date>");
+ JsonReader reader = new JsonReader(input, null, 1, schema, "");
+ VectorizedRowBatch batch = schema.createRowBatch(2);
+ assertEquals(true, reader.nextBatch(batch));
+ assertEquals(2, batch.size);
+ DateColumnVector cv = (DateColumnVector) batch.cols[0];
+ assertEquals(date1, LocalDate.ofEpochDay(cv.vector[0]));
+ assertEquals(date2, LocalDate.ofEpochDay(cv.vector[1]));
+
+ }
+
Review comment:
Can we please add some tests for Timestamp conversion as well? Ideally
covering all the conversion scenarios
##########
File path: java/tools/src/test/org/apache/orc/tools/convert/TestJsonReader.java
##########
@@ -72,4 +75,25 @@ public void testTimestampOffByOne() throws Exception {
assertEquals("1969-12-31 23:59:58.9999",
cv.asScratchTimestamp(5).toString());
}
+ @Test
+ public void testDateTypeSupport() throws IOException {
Review comment:
Can we add a case for null rows, repeating rows etc.?
##########
File path: java/tools/src/java/org/apache/orc/tools/convert/JsonReader.java
##########
@@ -144,16 +156,20 @@ public void convert(JsonElement value, ColumnVector vect,
int row) {
} else {
TimestampColumnVector vector = (TimestampColumnVector) vect;
TemporalAccessor temporalAccessor =
dateTimeFormatter.parseBest(value.getAsString(),
- ZonedDateTime.FROM, LocalDateTime.FROM);
+ ZonedDateTime::from, OffsetDateTime::from, LocalDateTime::from);
if (temporalAccessor instanceof ZonedDateTime) {
ZonedDateTime zonedDateTime = ((ZonedDateTime) temporalAccessor);
- Timestamp timestamp = new Timestamp(zonedDateTime.toEpochSecond() *
1000L);
- timestamp.setNanos(zonedDateTime.getNano());
+ Timestamp timestamp = Timestamp.from(zonedDateTime.toInstant());
+ vector.set(row, timestamp);
+ }
+ else if (temporalAccessor instanceof OffsetDateTime) {
+ OffsetDateTime offsetDateTime = (OffsetDateTime) temporalAccessor;
+ Timestamp timestamp = Timestamp.from(offsetDateTime.toInstant());
vector.set(row, timestamp);
- } else if (temporalAccessor instanceof LocalDateTime) {
+ }
+ else if (temporalAccessor instanceof LocalDateTime) {
Review comment:
same 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:
[email protected]
