Skip to content

PARQUET-1253: Support for new logical type representation #463

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

Merged
gszadovszky merged 12 commits into from
May 24, 2018
Merged

PARQUET-1253: Support for new logical type representation #463

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
244bb6a
PARQUET-1253: Support for new logical type representation
nkollar Mar 28, 2018
0a66346
refactor: move conversion logic to logical type classes
nkollar Mar 29, 2018
f8e2236
better (hopefully) name for new type
nkollar Mar 29, 2018
6b4ff74
Address code review comments
nkollar Apr 4, 2018
f801605
Deprecate old constructors, address comment related to builder
nkollar Apr 18, 2018
db30adb
Address latest comments
nkollar Apr 19, 2018
047feb9
Introduce type visitors for logical types, and address code review co…
nkollar Apr 24, 2018
d11a09c
Move factory methods to the interface for each logical type subclass
nkollar Apr 24, 2018
eb432f7
Incorporate new logical type parameters into schema language
nkollar May 8, 2018
3c426d9
Address latest code review comments
nkollar May 11, 2018
77f1d52
Make LogicalTypeToken enum and getType method package private
nkollar May 14, 2018
6e1ea5d
Make typeParametersAsString package private
nkollar May 16, 2018
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
Prev Previous commit
Next Next commit
Move factory methods to the interface for each logical type subclass
  • Loading branch information
@nkollar
nkollar committed Apr 24, 2018
commit d11a09cd733faae7cdf6452459cd949efcffb453
174 changes: 88 additions & 86 deletions parquet-column/src/main/java/org/apache/parquet/schema/LogicalTypeAnnotation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,63 +77,116 @@ static LogicalTypeAnnotation fromOriginalType(OriginalType originalType, Decimal
}
switch (originalType) {
case UTF8:
return StringLogicalTypeAnnotation.create();
return stringType();
case MAP:
return MapLogicalTypeAnnotation.create();
return mapType();
case DECIMAL:
int scale = (decimalMetadata == null ? 0 : decimalMetadata.getScale());
int precision = (decimalMetadata == null ? 0 : decimalMetadata.getPrecision());
return DecimalLogicalTypeAnnotation.create(scale, precision);
return decimalType(scale, precision);
case LIST:
return ListLogicalTypeAnnotation.create();
return listType();
case DATE:
return DateLogicalTypeAnnotation.create();
return dateType();
case INTERVAL:
return IntervalLogicalTypeAnnotation.create();
return intervalType();
case TIMESTAMP_MILLIS:
return TimestampLogicalTypeAnnotation.create(true, LogicalTypeAnnotation.TimeUnit.MILLIS);
return timestampType(true, LogicalTypeAnnotation.TimeUnit.MILLIS);
case TIMESTAMP_MICROS:
return TimestampLogicalTypeAnnotation.create(true, LogicalTypeAnnotation.TimeUnit.MICROS);
return timestampType(true, LogicalTypeAnnotation.TimeUnit.MICROS);
case TIME_MILLIS:
return TimeLogicalTypeAnnotation.create(true, LogicalTypeAnnotation.TimeUnit.MILLIS);
return timeType(true, LogicalTypeAnnotation.TimeUnit.MILLIS);
case TIME_MICROS:
return TimeLogicalTypeAnnotation.create(true, LogicalTypeAnnotation.TimeUnit.MICROS);
return timeType(true, LogicalTypeAnnotation.TimeUnit.MICROS);
case UINT_8:
return IntLogicalTypeAnnotation.create(8, false);
return intType(8, false);
case UINT_16:
return IntLogicalTypeAnnotation.create(16, false);
return intType(16, false);
case UINT_32:
return IntLogicalTypeAnnotation.create(32, false);
return intType(32, false);
case UINT_64:
return IntLogicalTypeAnnotation.create(64, false);
return intType(64, false);
case INT_8:
return IntLogicalTypeAnnotation.create(8, true);
return intType(8, true);
case INT_16:
return IntLogicalTypeAnnotation.create(16, true);
return intType(16, true);
case INT_32:
return IntLogicalTypeAnnotation.create(32, true);
return intType(32, true);
case INT_64:
return IntLogicalTypeAnnotation.create(64, true);
return intType(64, true);
case ENUM:
return EnumLogicalTypeAnnotation.create();
return enumType();
case JSON:
return JsonLogicalTypeAnnotation.create();
return jsonType();
case BSON:
return BsonLogicalTypeAnnotation.create();
return bsonType();
case MAP_KEY_VALUE:
return MapKeyValueTypeAnnotation.create();
return mapKeyValueType();
default:
throw new RuntimeException("Can't convert original type to logical type, unknown original type " + originalType);
}
}


static StringLogicalTypeAnnotation stringType() {
return StringLogicalTypeAnnotation.INSTANCE;
}

static MapLogicalTypeAnnotation mapType() {
return MapLogicalTypeAnnotation.INSTANCE;
}

static ListLogicalTypeAnnotation listType() {
return ListLogicalTypeAnnotation.INSTANCE;
}

static EnumLogicalTypeAnnotation enumType() {
return EnumLogicalTypeAnnotation.INSTANCE;
}

static DecimalLogicalTypeAnnotation decimalType(final int scale, final int precision) {
return new DecimalLogicalTypeAnnotation(scale, precision);
}

static DateLogicalTypeAnnotation dateType() {
return DateLogicalTypeAnnotation.INSTANCE;
}

static TimeLogicalTypeAnnotation timeType(final boolean isAdjustedToUTC, final TimeUnit unit) {
return new TimeLogicalTypeAnnotation(isAdjustedToUTC, unit);
}

static TimestampLogicalTypeAnnotation timestampType(final boolean isAdjustedToUTC, final TimeUnit unit) {
return new TimestampLogicalTypeAnnotation(isAdjustedToUTC, unit);
}

static IntLogicalTypeAnnotation intType(final int bitWidth, final boolean isSigned) {
Preconditions.checkArgument(
bitWidth == 8 || bitWidth == 16 || bitWidth == 32 || bitWidth == 64,
"Invalid bit width for integer logical type, " + bitWidth + " is not allowed, " +
"valid bit width values: 8, 16, 32, 64");
return new IntLogicalTypeAnnotation(bitWidth, isSigned);
}

static JsonLogicalTypeAnnotation jsonType() {
return JsonLogicalTypeAnnotation.INSTANCE;
}

static BsonLogicalTypeAnnotation bsonType() {
return BsonLogicalTypeAnnotation.INSTANCE;
}

static IntervalLogicalTypeAnnotation intervalType() {
Copy link
Contributor

@gszadovszky gszadovszky Apr 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want the user to be able to create an Interval or a MapKeyValue type? If they are only exist for backward compatibility reasons the factory method should not be public so users won't be able to create them.

Copy link
Contributor Author

@nandorKollar nandorKollar May 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think MapKeyValue is here only for backward compatibility, but Interval should be supported in the future. Since these factory methods are defined on an interface, I can't easily change the visibility to private/package protected. Should I change it to an abstract class?

Copy link
Contributor

@gszadovszky gszadovszky May 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interval is not supported in LogicalType currently, so I would not expose it yet. It is always possible to make a restricted access public but in the other way it would break backward compatibility.
Abstract class sounds reasonable to me.

return IntervalLogicalTypeAnnotation.INSTANCE;
}

static MapKeyValueTypeAnnotation mapKeyValueType() {
return MapKeyValueTypeAnnotation.INSTANCE;
}

class StringLogicalTypeAnnotation implements LogicalTypeAnnotation {
private static final StringLogicalTypeAnnotation INSTANCE = new StringLogicalTypeAnnotation();

public static LogicalTypeAnnotation create() {
return INSTANCE;
}

private StringLogicalTypeAnnotation() {
}

Expand Down Expand Up @@ -164,18 +217,14 @@ public boolean equals(Object obj) {

@Override
public int hashCode() {
// This type doesn't have any parameters, thus use class hashcode
// This type doesn't have any parameters, thus using class hashcode
return getClass().hashCode();
}
}

class MapLogicalTypeAnnotation implements LogicalTypeAnnotation {
private static final MapLogicalTypeAnnotation INSTANCE = new MapLogicalTypeAnnotation();

public static LogicalTypeAnnotation create() {
return INSTANCE;
}

private MapLogicalTypeAnnotation() {
}

Expand Down Expand Up @@ -206,18 +255,14 @@ public boolean equals(Object obj) {

@Override
public int hashCode() {
// This type doesn't have any parameters, thus use class hashcode
// This type doesn't have any parameters, thus using class hashcode
return getClass().hashCode();
}
}

class ListLogicalTypeAnnotation implements LogicalTypeAnnotation {
private static final ListLogicalTypeAnnotation INSTANCE = new ListLogicalTypeAnnotation();

public static LogicalTypeAnnotation create() {
return INSTANCE;
}

private ListLogicalTypeAnnotation() {
}

Expand Down Expand Up @@ -248,18 +293,14 @@ public boolean equals(Object obj) {

@Override
public int hashCode() {
// This type doesn't have any parameters, thus use class hashcode
// This type doesn't have any parameters, thus using class hashcode
return getClass().hashCode();
}
}

class EnumLogicalTypeAnnotation implements LogicalTypeAnnotation {
private static final EnumLogicalTypeAnnotation INSTANCE = new EnumLogicalTypeAnnotation();

public static LogicalTypeAnnotation create() {
return INSTANCE;
}

private EnumLogicalTypeAnnotation() {
}

Expand Down Expand Up @@ -290,7 +331,7 @@ public boolean equals(Object obj) {

@Override
public int hashCode() {
// This type doesn't have any parameters, thus use class hashcode
// This type doesn't have any parameters, thus using class hashcode
return getClass().hashCode();
}
}
Expand All @@ -299,10 +340,6 @@ class DecimalLogicalTypeAnnotation implements LogicalTypeAnnotation {
private final int scale;
private final int precision;

public static LogicalTypeAnnotation create(int scale, int precision) {
return new DecimalLogicalTypeAnnotation(scale, precision);
}

private DecimalLogicalTypeAnnotation(int scale, int precision) {
this.scale = scale;
this.precision = precision;
Expand Down Expand Up @@ -354,10 +391,6 @@ public int hashCode() {
class DateLogicalTypeAnnotation implements LogicalTypeAnnotation {
private static final DateLogicalTypeAnnotation INSTANCE = new DateLogicalTypeAnnotation();

public static LogicalTypeAnnotation create() {
return INSTANCE;
}

private DateLogicalTypeAnnotation() {
}

Expand Down Expand Up @@ -388,7 +421,7 @@ public boolean equals(Object obj) {

@Override
public int hashCode() {
// This type doesn't have any parameters, thus use class hashcode
// This type doesn't have any parameters, thus using class hashcode
return getClass().hashCode();
}
}
Expand All @@ -413,10 +446,6 @@ class TimeLogicalTypeAnnotation implements LogicalTypeAnnotation {
private final boolean isAdjustedToUTC;
private final TimeUnit unit;

public static LogicalTypeAnnotation create(boolean isAdjustedToUTC, TimeUnit unit) {
return new TimeLogicalTypeAnnotation(isAdjustedToUTC, unit);
}

private TimeLogicalTypeAnnotation(boolean isAdjustedToUTC, TimeUnit unit) {
this.isAdjustedToUTC = isAdjustedToUTC;
this.unit = unit;
Expand Down Expand Up @@ -483,10 +512,6 @@ class TimestampLogicalTypeAnnotation implements LogicalTypeAnnotation {
private final boolean isAdjustedToUTC;
private final TimeUnit unit;

public static LogicalTypeAnnotation create(boolean isAdjustedToUTC, TimeUnit unit) {
return new TimestampLogicalTypeAnnotation(isAdjustedToUTC, unit);
}

private TimestampLogicalTypeAnnotation(boolean isAdjustedToUTC, TimeUnit unit) {
this.isAdjustedToUTC = isAdjustedToUTC;
this.unit = unit;
Expand Down Expand Up @@ -553,13 +578,6 @@ class IntLogicalTypeAnnotation implements LogicalTypeAnnotation {
private final int bitWidth;
private final boolean isSigned;

public static LogicalTypeAnnotation create(int bitWidth, boolean isSigned) {
Preconditions.checkArgument(
bitWidth == 8 || bitWidth == 16 || bitWidth == 32 || bitWidth == 64,
"Invalid bit width for integer logical type, " + bitWidth + " is not allowed, " +
"valid bit width values: 8, 16, 32, 64");
return new IntLogicalTypeAnnotation(bitWidth, isSigned);
}

private IntLogicalTypeAnnotation(int bitWidth, boolean isSigned) {
this.bitWidth = bitWidth;
Expand Down Expand Up @@ -642,10 +660,6 @@ public int hashCode() {
class JsonLogicalTypeAnnotation implements LogicalTypeAnnotation {
private static final JsonLogicalTypeAnnotation INSTANCE = new JsonLogicalTypeAnnotation();

public static LogicalTypeAnnotation create() {
return INSTANCE;
}

private JsonLogicalTypeAnnotation() {
}

Expand Down Expand Up @@ -676,18 +690,14 @@ public boolean equals(Object obj) {

@Override
public int hashCode() {
// This type doesn't have any parameters, thus use class hashcode
// This type doesn't have any parameters, thus using class hashcode
return getClass().hashCode();
}
}

class BsonLogicalTypeAnnotation implements LogicalTypeAnnotation {
private static final BsonLogicalTypeAnnotation INSTANCE = new BsonLogicalTypeAnnotation();

public static LogicalTypeAnnotation create() {
return INSTANCE;
}

private BsonLogicalTypeAnnotation() {
}

Expand Down Expand Up @@ -718,7 +728,7 @@ public boolean equals(Object obj) {

@Override
public int hashCode() {
// This type doesn't have any parameters, thus use class hashcode
// This type doesn't have any parameters, thus using class hashcode
return getClass().hashCode();
}
}
Expand All @@ -729,10 +739,6 @@ public int hashCode() {
class IntervalLogicalTypeAnnotation implements LogicalTypeAnnotation {
Copy link
Contributor

@gszadovszky gszadovszky Apr 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is fake type as INTERVAL is not supported in LogicalTypes currently. I can see the purpose though. I would suggest adding some comments to make the purpose clear.

private static IntervalLogicalTypeAnnotation INSTANCE = new IntervalLogicalTypeAnnotation();

public static LogicalTypeAnnotation create() {
return INSTANCE;
}

private IntervalLogicalTypeAnnotation() {
}

Expand Down Expand Up @@ -763,7 +769,7 @@ public boolean equals(Object obj) {

@Override
public int hashCode() {
// This type doesn't have any parameters, thus use class hashcode
// This type doesn't have any parameters, thus using class hashcode
return getClass().hashCode();
}
}
Expand All @@ -774,10 +780,6 @@ public int hashCode() {
class MapKeyValueTypeAnnotation implements LogicalTypeAnnotation {
private static MapKeyValueTypeAnnotation INSTANCE = new MapKeyValueTypeAnnotation();

public static LogicalTypeAnnotation create() {
return INSTANCE;
}

private MapKeyValueTypeAnnotation() {
}

Expand Down Expand Up @@ -808,7 +810,7 @@ public boolean equals(Object obj) {

@Override
public int hashCode() {
// This type doesn't have any parameters, thus use class hashcode
// This type doesn't have any parameters, thus using class hashcode
return getClass().hashCode();
}
}
Expand Down
Loading