Skip to content

[multistage] Support TIMESTAMP type and date ops functions #11350

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
xiangfu0 merged 6 commits into from
Aug 17, 2023
Merged

[multistage] Support TIMESTAMP type and date ops functions #11350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
84d731f
Convert TIMESTAMP type at reduce type
xiangfu0 Aug 15, 2023
8c9ad60
Use DataSchema conversion functions
xiangfu0 Aug 15, 2023
c9e4c1b
Register fromDateTime function
xiangfu0 Aug 16, 2023
494f6a1
Support TIMESTAMP literal
xiangfu0 Aug 16, 2023
c82697e
convert timestamp literal value from GregorianCalendar type
xiangfu0 Aug 16, 2023
a386ac2
Address comments
xiangfu0 Aug 17, 2023
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
20 changes: 20 additions & 0 deletions pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@
import org.apache.pinot.spi.data.DateTimeFormatSpec;


/**
* The {@code TransformFunctionType} enum represents all the transform functions supported by Calcite SQL parser in
* v2 engine.
* TODO: Add support for scalar functions auto registration.
*/
public enum TransformFunctionType {
// arithmetic functions for single-valued columns
ADD("add", "plus"),
Expand Down Expand Up @@ -124,6 +129,21 @@ public enum TransformFunctionType {
SqlTypeFamily.CHARACTER),
ordinal -> ordinal > 1)),

FROMDATETIME("fromDateTime", ReturnTypes.TIMESTAMP_NULLABLE,
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang Aug 17, 2023

Choose a reason for hiding this comment

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

Please add a TODO to auto generate signature for scalar function. We should not manually add this for scalar function

OperandTypes.family(ImmutableList.of(SqlTypeFamily.CHARACTER, SqlTypeFamily.CHARACTER, SqlTypeFamily.CHARACTER),
ordinal -> ordinal > 1)),

TODATETIME("toDateTime", ReturnTypes.VARCHAR_2000_NULLABLE,
OperandTypes.family(ImmutableList.of(SqlTypeFamily.ANY, SqlTypeFamily.CHARACTER, SqlTypeFamily.CHARACTER),
ordinal -> ordinal > 1)),

TIMESTAMPADD("timestampAdd", ReturnTypes.TIMESTAMP_NULLABLE,
OperandTypes.family(ImmutableList.of(SqlTypeFamily.CHARACTER, SqlTypeFamily.NUMERIC, SqlTypeFamily.ANY)),
"dateAdd"),

TIMESTAMPDIFF("timestampDiff", ReturnTypes.BIGINT_NULLABLE,
OperandTypes.family(ImmutableList.of(SqlTypeFamily.CHARACTER, SqlTypeFamily.ANY, SqlTypeFamily.ANY)), "dateDiff"),

YEAR("year"),
YEAR_OF_WEEK("yearOfWeek", "yow"),
QUARTER("quarter"),
Expand Down
6 changes: 3 additions & 3 deletions pinot-common/src/main/java/org/apache/pinot/common/request/context/LiteralContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,14 @@ public LiteralContext(Literal literal) {
Pair<FieldSpec.DataType, Object> typeAndValue =
inferLiteralDataTypeAndValue(literal.getFieldValue().toString());
_type = typeAndValue.getLeft();
_value = typeAndValue.getRight();
if (_type == FieldSpec.DataType.BIG_DECIMAL) {
_bigDecimalValue = (BigDecimal) _value;
_bigDecimalValue = (BigDecimal) typeAndValue.getRight();
} else if (_type == FieldSpec.DataType.TIMESTAMP) {
_bigDecimalValue = PinotDataType.TIMESTAMP.toBigDecimal(Timestamp.valueOf(_value.toString()));
_bigDecimalValue = PinotDataType.TIMESTAMP.toBigDecimal(typeAndValue.getRight());
} else {
_bigDecimalValue = BigDecimal.ZERO;
}
_value = literal.getFieldValue().toString();
break;
case NULL_VALUE:
_type = FieldSpec.DataType.UNKNOWN;
Expand Down
11 changes: 10 additions & 1 deletion pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,10 @@ public Serializable convert(Object value) {
case BOOLEAN:
return ((Number) value).intValue() == 1;
case TIMESTAMP:
return new Timestamp((long) value);
if (value instanceof Timestamp) {
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang Aug 17, 2023

Choose a reason for hiding this comment

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

Does this still apply? Ideally we should always have internal format value. If we break that contract, BOOLEAN will also break

return (Timestamp) value;
}
return new Timestamp(((Number) value).longValue());
case STRING:
case JSON:
return value.toString();
Expand Down Expand Up @@ -416,8 +419,14 @@ public Serializable convertAndFormat(Object value) {
case BIG_DECIMAL:
return (BigDecimal) value;
case BOOLEAN:
if (value instanceof Boolean) {
return (boolean) value;
}
return ((Number) value).intValue() == 1;
case TIMESTAMP:
if (value instanceof Timestamp) {
return value.toString();
}
return new Timestamp((long) value).toString();
case STRING:
case JSON:
Expand Down
13 changes: 11 additions & 2 deletions pinot-core/src/main/java/org/apache/pinot/core/common/datablock/DataBlockBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,19 @@ public static RowDataBlock buildFromRows(List<Object[]> rows, DataSchema dataSch
setColumn(rowBuilder, byteBuffer, (BigDecimal) value);
break;
case BOOLEAN:
byteBuffer.putInt(((Boolean) value) ? 1 : 0);
if (value instanceof Boolean) {
byteBuffer.putInt(((Boolean) value) ? 1 : 0);
} else {
byteBuffer.putInt(((Number) value).intValue() > 0 ? 1 : 0);
}
break;
case TIMESTAMP:
byteBuffer.putLong(((Timestamp) value).getTime());
// Certain non strong typed functions in v2 might return long value instead of Timestamp.
if (value instanceof Long) {
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang Aug 17, 2023

Choose a reason for hiding this comment

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

Are we going to hit both? If so, can you add some java doc explaining when will we hit both? If we can hit both, then BOOLEAN will also break

Copy link
Contributor Author

@xiangfu0 xiangfu0 Aug 17, 2023

Choose a reason for hiding this comment

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

Certain non strong typed functions in v2 might return long value then in datablock build, we need to do the conversion.

byteBuffer.putLong((long) value);
} else {
byteBuffer.putLong(((Timestamp) value).getTime());
}
break;
case STRING:
setColumn(rowBuilder, byteBuffer, (String) value);
Expand Down
8 changes: 8 additions & 0 deletions ...ava/org/apache/pinot/core/operator/transform/function/ScalarTransformFunctionWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,18 @@ public void init(List<TransformFunction> arguments, Map<String, ColumnContext> c
parameterTypes[i].convert(literalTransformFunction.getDoubleLiteral(), PinotDataType.DOUBLE);
break;
case BIG_DECIMAL:
if (parameterTypes[i] == PinotDataType.STRING) {
_scalarArguments[i] = literalTransformFunction.getStringLiteral();
break;
}
_scalarArguments[i] =
parameterTypes[i].convert(literalTransformFunction.getBigDecimalLiteral(), PinotDataType.BIG_DECIMAL);
break;
case TIMESTAMP:
if (parameterTypes[i] == PinotDataType.STRING) {
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang Aug 17, 2023

Choose a reason for hiding this comment

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

I feel this applies to all data types

Copy link
Contributor Author

@xiangfu0 xiangfu0 Aug 17, 2023

Choose a reason for hiding this comment

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

For other data types, only BIG_DECIMAL is the inferable type. So just need to add support for that.

_scalarArguments[i] = literalTransformFunction.getStringLiteral();
break;
}
_scalarArguments[i] =
parameterTypes[i].convert(literalTransformFunction.getLongLiteral(), PinotDataType.TIMESTAMP);
break;
Expand Down
Loading