Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use long for representing date value in Accumulo #11055

Merged
merged 2 commits into from
Feb 18, 2022
Merged

Use long for representing date value in Accumulo #11055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2799ee
Use long for representing date value in Accumulo
ebyhr Feb 16, 2022
8748eb4
empty
ebyhr Feb 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions plugin/trino-accumulo/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,6 @@
<artifactId>validation-api</artifactId>
</dependency>

<dependency>
<groupId>joda-time</groupId>
<artifactId>joda-time</artifactId>
</dependency>

<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
Expand Down
3 changes: 1 addition & 2 deletions plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/io/AccumuloRecordCursor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import static io.trino.spi.type.TinyintType.TINYINT;
import static java.lang.String.format;
import static java.util.Objects.requireNonNull;
import static java.util.concurrent.TimeUnit.MILLISECONDS;

/**
* Implementation of Trino RecordCursor, responsible for iterating over a Trino split,
Expand Down Expand Up @@ -203,7 +202,7 @@ public long getLong(int field)
return serializer.getLong(fieldToColumnName[field]);
}
if (type.equals(DATE)) {
return MILLISECONDS.toDays(serializer.getDate(fieldToColumnName[field]).getTime());
return serializer.getDate(fieldToColumnName[field]);
}
if (type.equals(INTEGER)) {
return serializer.getInt(fieldToColumnName[field]);
Expand Down
11 changes: 5 additions & 6 deletions plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/model/Field.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,8 @@
import io.trino.spi.type.Type;
import io.trino.spi.type.VarcharType;

import java.sql.Date;
import java.sql.Time;
import java.sql.Timestamp;
import java.time.LocalDate;
import java.util.Arrays;
import java.util.Objects;

Expand Down Expand Up @@ -87,7 +85,7 @@ else if (type.equals(BOOLEAN)) {
this.value = field.getBoolean();
}
else if (type.equals(DATE)) {
this.value = new Date(field.getDate().getTime());
this.value = field.getDate();
}
else if (type.equals(DOUBLE)) {
this.value = field.getDouble();
Expand Down Expand Up @@ -146,9 +144,9 @@ public Byte getByte()
return (Byte) value;
}

public Date getDate()
public long getDate()
{
return (Date) value;
return (Long) value;
}

public Double getDouble()
Expand Down Expand Up @@ -316,7 +314,8 @@ private static Object convert(Object value, Type type)
}

if (type.equals(DATE)) {
return Date.valueOf(LocalDate.ofEpochDay((long) value));
// long
return value;
}

if (type.equals(DOUBLE)) {
Expand Down
7 changes: 3 additions & 4 deletions ...no-accumulo/src/main/java/io/trino/plugin/accumulo/serializers/AccumuloRowSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.apache.accumulo.core.data.Value;
import org.apache.hadoop.io.Text;

import java.sql.Date;
import java.sql.Time;
import java.sql.Timestamp;
import java.util.HashMap;
Expand Down Expand Up @@ -168,15 +167,15 @@ static AccumuloRowSerializer getDefault()
* @param name Column name
* @return Date value
*/
Date getDate(String name);
long getDate(String name);

/**
* Encode the given Date value into the given Text object.
*
* @param text Text object to set
* @param value Value to encode
*/
void setDate(Text text, Date value);
void setDate(Text text, long value);

/**
* Gets the Double value of the given Trino column.
Expand Down Expand Up @@ -366,7 +365,7 @@ static AccumuloRowSerializer getDefault()
* </tr>
* <tr>
* <td>DATE</td>
* <td>java.sql.Date, Long</td>
* <td>long</td>
* </tr>
* <tr>
* <td>DOUBLE</td>
Expand Down
7 changes: 3 additions & 4 deletions ...o-accumulo/src/main/java/io/trino/plugin/accumulo/serializers/LexicoderRowSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
import static io.trino.spi.type.TinyintType.TINYINT;
import static io.trino.spi.type.VarbinaryType.VARBINARY;
import static io.trino.spi.type.VarcharType.VARCHAR;
import static java.util.concurrent.TimeUnit.DAYS;
import static java.util.concurrent.TimeUnit.MILLISECONDS;

/**
Expand Down Expand Up @@ -189,13 +188,13 @@ public void setByte(Text text, Byte value)
}

@Override
public Date getDate(String name)
public long getDate(String name)
{
return new Date(DAYS.toMillis(decode(BIGINT, getFieldValue(name))));
return decode(BIGINT, getFieldValue(name));
}

@Override
public void setDate(Text text, Date value)
public void setDate(Text text, long value)
{
text.set(encode(DATE, value));
}
Expand Down
13 changes: 5 additions & 8 deletions ...rino-accumulo/src/main/java/io/trino/plugin/accumulo/serializers/StringRowSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.accumulo.core.data.Value;
import org.apache.hadoop.io.Text;

import java.sql.Date;
import java.sql.Time;
import java.sql.Timestamp;
import java.util.HashMap;
Expand All @@ -45,8 +44,6 @@
import static io.trino.spi.type.VarcharType.VARCHAR;
import static java.lang.String.format;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.concurrent.TimeUnit.DAYS;
import static java.util.concurrent.TimeUnit.MILLISECONDS;

/**
* Implementation of {@link StringRowSerializer} that encodes and decodes Trino column values as human-readable String objects.
Expand Down Expand Up @@ -161,15 +158,15 @@ public void setByte(Text text, Byte value)
}

@Override
public Date getDate(String name)
public long getDate(String name)
{
return new Date(DAYS.toMillis(Long.parseLong(getFieldValue(name))));
return Long.parseLong(getFieldValue(name));
}

@Override
public void setDate(Text text, Date value)
public void setDate(Text text, long value)
{
text.set(Long.toString(MILLISECONDS.toDays(value.getTime())).getBytes(UTF_8));
text.set(Long.toString(value).getBytes(UTF_8));
}

@Override
Expand Down Expand Up @@ -317,7 +314,7 @@ else if (type.equals(BOOLEAN)) {
setBoolean(text, value.equals(Boolean.TRUE));
}
else if (type.equals(DATE)) {
setDate(text, (Date) value);
setDate(text, (long) value);
}
else if (type.equals(DOUBLE)) {
setDouble(text, (Double) value);
Expand Down
33 changes: 0 additions & 33 deletions plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/TestAccumuloConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,30 +193,6 @@ public void testInsertDuplicateRows()
}
}

@Test
@Override
public void testCreateTableAsSelectNegativeDate()
{
// TODO (https://github.com/trinodb/trino/issues/10208) Fix negative date handling.
assertThatThrownBy(super::testCreateTableAsSelectNegativeDate)
.isInstanceOf(AssertionError.class)
.hasMessageContaining("" +
"Actual rows (up to 100 of 1 extra rows shown, 1 rows in total):\n" +
" [-0002-12-31]");
}

@Test
@Override
public void testInsertNegativeDate()
{
// TODO (https://github.com/trinodb/trino/issues/10208) Fix negative date handling.
assertThatThrownBy(super::testInsertNegativeDate)
.isInstanceOf(AssertionError.class)
.hasMessageContaining("" +
"Actual rows (up to 100 of 1 extra rows shown, 1 rows in total):\n" +
" [-0002-12-31]");
}

@Override
public void testShowColumns()
{
Expand Down Expand Up @@ -321,15 +297,6 @@ public void testShowCreateTable()
protected Optional<DataMappingTestSetup> filterDataMappingSmokeTestData(DataMappingTestSetup dataMappingTestSetup)
{
String typeName = dataMappingTestSetup.getTrinoTypeName();
if (typeName.equals("date")) {
// TODO (https://github.com/trinodb/trino/issues/10074) Investigate why this test case fails
if (dataMappingTestSetup.getSampleValueLiteral().equals("DATE '0001-01-01'")
|| dataMappingTestSetup.getSampleValueLiteral().equals("DATE '1582-10-04'")
|| dataMappingTestSetup.getSampleValueLiteral().equals("DATE '1582-10-05'")
|| dataMappingTestSetup.getSampleValueLiteral().equals("DATE '1582-10-14'")) {
return Optional.empty();
}
}
if (typeName.startsWith("decimal(")
|| typeName.equals("timestamp(3) with time zone")
|| typeName.startsWith("char(")) {
Expand Down
4 changes: 2 additions & 2 deletions plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/model/TestField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
import io.trino.spi.type.TypeSignatureParameter;
import org.testng.annotations.Test;

import java.sql.Date;
import java.sql.Time;
import java.sql.Timestamp;
import java.time.LocalDate;
import java.util.GregorianCalendar;

import static io.airlift.slice.Slices.utf8Slice;
Expand Down Expand Up @@ -91,7 +91,7 @@ public void testBoolean()
public void testDate()
{
Type type = DATE;
Date expected = new Date(new GregorianCalendar(1999, 0, 1).getTime().getTime());
long expected = LocalDate.parse("1999-01-01").toEpochDay();
Field f1 = new Field(10592L, type);
assertEquals(f1.getDate(), expected);
assertEquals(f1.getObject(), expected);
Expand Down
12 changes: 4 additions & 8 deletions ...src/test/java/io/trino/plugin/accumulo/serializers/AbstractTestAccumuloRowSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,11 @@
import org.apache.accumulo.core.data.Key;
import org.apache.accumulo.core.data.Mutation;
import org.apache.accumulo.core.data.Value;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.testng.annotations.Test;

import java.sql.Date;
import java.sql.Time;
import java.sql.Timestamp;
import java.time.LocalDate;
import java.util.AbstractMap.SimpleImmutableEntry;
import java.util.List;
import java.util.Map;
Expand All @@ -48,7 +46,6 @@
import static io.trino.spi.type.VarcharType.VARCHAR;
import static io.trino.type.InternalTypeManager.TESTING_TYPE_MANAGER;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.testng.Assert.assertEquals;

public abstract class AbstractTestAccumuloRowSerializer
Expand Down Expand Up @@ -104,15 +101,14 @@ public void testBoolean()
public void testDate()
throws Exception
{
Date expected = new Date(new DateTime(2001, 2, 3, 4, 5, 6, DateTimeZone.UTC).getMillis());
long expected = LocalDate.parse("2001-02-03").toEpochDay();
AccumuloRowSerializer serializer = serializerClass.getConstructor().newInstance();
byte[] data = serializer.encode(DATE, expected);

deserializeData(serializer, data);
Date actual = serializer.getDate(COLUMN_NAME);
long actual = serializer.getDate(COLUMN_NAME);

// Convert milliseconds to days so they can be compared regardless of the time of day
assertEquals(MILLISECONDS.toDays(actual.getTime()), MILLISECONDS.toDays(expected.getTime()));
assertEquals(actual, expected);
}

@Test
Expand Down