Skip to content

Commit 7b43752

Browse files
nielspardonvbarua
nielspardon
authored and
vbarua
committed
feat(isthmus): migrate to PrecisionTime / PrecisionTimestamp (#721)
This PR migrates isthmus to use PrecisionTime / PrecisionTimestamp instead of the deprecated Time / Timestamp types. Signed-off-by: Niels Pardon <par@zurich.ibm.com>
1 parent 724aa22 commit 7b43752

9 files changed

Lines changed: 109 additions & 31 deletions

File tree

‎core/src/main/java/io/substrait/expression/ExpressionCreator.java‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import java.nio.ByteBuffer;
1111
import java.time.Instant;
1212
import java.time.LocalDateTime;
13+
import java.time.LocalTime;
1314
import java.time.ZoneOffset;
1415
import java.util.Arrays;
1516
import java.util.List;
@@ -89,6 +90,23 @@ public static Expression.PrecisionTimeLiteral precisionTime(
8990
.build();
9091
}
9192

93+
/**
94+
* Creates a precision time literal from a LocalTime value.
95+
*
96+
* <p>This method converts a Java {@link LocalTime} to a Substrait precision time literal with
97+
* nanosecond precision (precision = 9). The time value is represented as nanoseconds since
98+
* midnight (00:00:00).
99+
*
100+
* @param nullable whether the literal can be null
101+
* @param value the LocalTime value to convert
102+
* @return a PrecisionTimeLiteral with nanosecond precision representing the given time
103+
* @see #precisionTime(boolean, long, int) for creating precision time with custom precision
104+
*/
105+
public static Expression.PrecisionTimeLiteral precisionTime(boolean nullable, LocalTime value) {
106+
long epochNano = value.toNanoOfDay();
107+
return precisionTime(nullable, epochNano, 9);
108+
}
109+
92110
/**
93111
* @deprecated Timestamp is deprecated in favor of PrecisionTimestamp
94112
*/
@@ -104,7 +122,7 @@ public static Expression.TimestampLiteral timestamp(boolean nullable, long value
104122
public static Expression.TimestampLiteral timestamp(boolean nullable, LocalDateTime value) {
105123
long epochMicro =
106124
TimeUnit.SECONDS.toMicros(value.toEpochSecond(ZoneOffset.UTC))
107-
+ TimeUnit.NANOSECONDS.toMicros(value.toLocalTime().getNano());
125+
+ TimeUnit.NANOSECONDS.toMicros(value.getNano());
108126
return timestamp(nullable, epochMicro);
109127
}
110128

@@ -155,6 +173,26 @@ public static Expression.PrecisionTimestampLiteral precisionTimestamp(
155173
.build();
156174
}
157175

176+
/**
177+
* Creates a precision timestamp literal from a LocalDateTime value.
178+
*
179+
* <p>This method converts a Java {@link LocalDateTime} to a Substrait precision timestamp literal
180+
* with nanosecond precision (precision = 9). The timestamp value is represented as nanoseconds
181+
* since the Unix epoch (1970-01-01 00:00:00 UTC), assuming the LocalDateTime is in UTC timezone.
182+
*
183+
* @param nullable whether the literal can be null
184+
* @param value the LocalDateTime value to convert (interpreted as UTC)
185+
* @return a PrecisionTimestampLiteral with nanosecond precision representing the given timestamp
186+
* @see #precisionTimestamp(boolean, long, int) for creating precision timestamp with custom
187+
* precision
188+
*/
189+
public static Expression.PrecisionTimestampLiteral precisionTimestamp(
190+
boolean nullable, LocalDateTime value) {
191+
long epochNano =
192+
TimeUnit.SECONDS.toNanos(value.toEpochSecond(ZoneOffset.UTC)) + value.getNano();
193+
return precisionTimestamp(nullable, epochNano, 9);
194+
}
195+
158196
public static Expression.PrecisionTimestampTZLiteral precisionTimestampTZ(
159197
boolean nullable, long value, int precision) {
160198
return Expression.PrecisionTimestampTZLiteral.builder()

‎isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ private Type toSubstrait(RelDataType type, List<String> names) {
106106
case DATE:
107107
return creator.DATE;
108108
case TIME:
109-
return creator.TIME;
109+
return creator.precisionTime(type.getPrecision());
110110
case TIMESTAMP:
111111
return creator.precisionTimestamp(type.getPrecision());
112112
case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
@@ -256,7 +256,7 @@ public RelDataType visit(Type.Timestamp expr) {
256256
public RelDataType visit(Type.PrecisionTime expr) {
257257
int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIME);
258258
if (expr.precision() > maxPrecision) {
259-
throw new UnsupportedOperationException(
259+
throw new IllegalArgumentException(
260260
String.format(
261261
"unsupported precision_time precision %s, max precision in Calcite type system is set to %s",
262262
expr.precision(), maxPrecision));
@@ -268,7 +268,7 @@ public RelDataType visit(Type.PrecisionTime expr) {
268268
public RelDataType visit(Type.PrecisionTimestamp expr) {
269269
int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP);
270270
if (expr.precision() > maxPrecision) {
271-
throw new UnsupportedOperationException(
271+
throw new IllegalArgumentException(
272272
String.format(
273273
"unsupported precision_timestamp precision %s, max precision in Calcite type system is set to %s",
274274
expr.precision(), maxPrecision));
@@ -281,7 +281,7 @@ public RelDataType visit(Type.PrecisionTimestampTZ expr) throws RuntimeException
281281
int maxPrecision =
282282
typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE);
283283
if (expr.precision() > maxPrecision) {
284-
throw new UnsupportedOperationException(
284+
throw new IllegalArgumentException(
285285
String.format(
286286
"unsupported precision_timestamp_tz precision %s, max precision in Calcite type system is set to %s",
287287
expr.precision(), maxPrecision));

‎isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import org.apache.calcite.sql.SqlOperator;
5454
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
5555
import org.apache.calcite.sql.parser.SqlParserPos;
56+
import org.apache.calcite.sql.type.SqlTypeName;
5657
import org.apache.calcite.util.TimeString;
5758
import org.apache.calcite.util.TimestampString;
5859

@@ -212,9 +213,16 @@ public RexNode visit(Expression.TimeLiteral expr, Context context) throws Runtim
212213

213214
@Override
214215
public RexNode visit(PrecisionTimeLiteral expr, Context context) throws RuntimeException {
215-
return rexBuilder.makeLiteral(
216-
createTimeString(expr.value(), expr.precision()),
217-
typeConverter.toCalcite(typeFactory, expr.getType()));
216+
int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIME);
217+
if (expr.precision() > maxPrecision) {
218+
throw new IllegalArgumentException(
219+
String.format(
220+
"unsupported precision_time precision %s, max precision in Calcite type system is set to %s",
221+
expr.precision(), maxPrecision));
222+
}
223+
224+
return rexBuilder.makeTimeLiteral(
225+
createTimeString(expr.value(), expr.precision()), expr.precision());
218226
}
219227

220228
protected TimeString createTimeString(long value, int precision) {
@@ -245,7 +253,7 @@ protected TimeString createTimeString(long value, int precision) {
245253
.withNanos(fracSecondsInNano);
246254
}
247255
default:
248-
throw new UnsupportedOperationException(
256+
throw new IllegalArgumentException(
249257
String.format("Cannot handle PrecisionTime with precision %d.", precision));
250258
}
251259
}
@@ -278,13 +286,28 @@ public RexNode visit(TimestampTZLiteral expr, Context context) throws RuntimeExc
278286

279287
@Override
280288
public RexNode visit(PrecisionTimestampLiteral expr, Context context) throws RuntimeException {
281-
return rexBuilder.makeLiteral(
282-
getTimestampString(expr.value(), expr.precision()),
283-
typeConverter.toCalcite(typeFactory, expr.getType()));
289+
int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP);
290+
if (expr.precision() > maxPrecision) {
291+
throw new IllegalArgumentException(
292+
String.format(
293+
"unsupported precision_timestamp precision %s, max precision in Calcite type system is set to %s",
294+
expr.precision(), maxPrecision));
295+
}
296+
297+
return rexBuilder.makeTimestampLiteral(
298+
getTimestampString(expr.value(), expr.precision()), expr.precision());
284299
}
285300

286301
@Override
287302
public RexNode visit(PrecisionTimestampTZLiteral expr, Context context) throws RuntimeException {
303+
int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP_TZ);
304+
if (expr.precision() > maxPrecision) {
305+
throw new IllegalArgumentException(
306+
String.format(
307+
"unsupported precision_timestamp precision %s, max precision in Calcite type system is set to %s",
308+
expr.precision(), maxPrecision));
309+
}
310+
288311
return rexBuilder.makeLiteral(
289312
getTimestampString(expr.value(), expr.precision()),
290313
typeConverter.toCalcite(typeFactory, expr.getType()));
@@ -322,7 +345,7 @@ private TimestampString getTimestampString(long value, int precision) {
322345
.withNanos(fracSecondsInNano);
323346
}
324347
default:
325-
throw new UnsupportedOperationException(
348+
throw new IllegalArgumentException(
326349
String.format("Cannot handle PrecisionTimestamp with precision %d.", precision));
327350
}
328351
}

‎isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -170,16 +170,23 @@ public Expression.Literal convert(RexLiteral literal, boolean nullable) {
170170
{
171171
TimeString time = literal.getValueAs(TimeString.class);
172172
LocalTime localTime = LocalTime.parse(time.toString(), CALCITE_LOCAL_TIME_FORMATTER);
173-
return ExpressionCreator.time(
174-
nullable, TimeUnit.NANOSECONDS.toMicros(localTime.toNanoOfDay()));
173+
// Calcite supports up to microsecond precision (6), convert nanoseconds to microseconds
174+
long nanos = localTime.toNanoOfDay();
175+
long micros = TimeUnit.NANOSECONDS.toMicros(nanos);
176+
return ExpressionCreator.precisionTime(nullable, micros, 6);
175177
}
176178
case TIMESTAMP:
177179
case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
178180
{
179181
TimestampString timestamp = literal.getValueAs(TimestampString.class);
180-
LocalDateTime ldt =
182+
LocalDateTime localDateTime =
181183
LocalDateTime.parse(timestamp.toString(), CALCITE_LOCAL_DATETIME_FORMATTER);
182-
return ExpressionCreator.timestamp(nullable, ldt);
184+
// Calcite supports up to microsecond precision (6), convert nanoseconds to microseconds
185+
long epochNanos =
186+
TimeUnit.SECONDS.toNanos(localDateTime.toEpochSecond(java.time.ZoneOffset.UTC))
187+
+ localDateTime.toLocalTime().getNano();
188+
long epochMicros = TimeUnit.NANOSECONDS.toMicros(epochNanos);
189+
return ExpressionCreator.precisionTimestamp(nullable, epochMicros, 6);
183190
}
184191
case INTERVAL_YEAR:
185192
case INTERVAL_YEAR_MONTH:

‎isthmus/src/test/java/io/substrait/isthmus/CalciteCallTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class CalciteCallTest extends CalciteObjs {
3535
@Test
3636
void extract() {
3737
test(
38-
"extract:req_ts",
38+
"extract:req_pts",
3939
rex.makeCall(
4040
t(SqlTypeName.INTEGER),
4141
SqlStdOperatorTable.EXTRACT,

‎isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import io.substrait.expression.Expression.IntervalDayLiteral;
99
import io.substrait.expression.Expression.IntervalYearLiteral;
1010
import io.substrait.expression.Expression.Literal;
11-
import io.substrait.expression.Expression.TimestampLiteral;
11+
import io.substrait.expression.Expression.PrecisionTimestampLiteral;
1212
import io.substrait.expression.ExpressionCreator;
1313
import io.substrait.extension.DefaultExtensionCatalog;
1414
import io.substrait.extension.SimpleExtension;
@@ -21,6 +21,8 @@
2121
import java.math.BigDecimal;
2222
import java.nio.charset.StandardCharsets;
2323
import java.time.LocalDate;
24+
import java.time.LocalDateTime;
25+
import java.time.ZoneOffset;
2426
import java.util.Arrays;
2527
import java.util.Collections;
2628
import java.util.List;
@@ -106,7 +108,7 @@ void tBinary() {
106108
@Test
107109
void tTime() {
108110
bitest(
109-
ExpressionCreator.time(false, (14L * 60 * 60 + 22 * 60 + 47) * 1000 * 1000),
111+
ExpressionCreator.precisionTime(false, (14L * 60 * 60 + 22 * 60 + 47) * 1000 * 1000, 6),
110112
rex.makeTimeLiteral(new TimeString(14, 22, 47), 6));
111113
}
112114

@@ -122,7 +124,8 @@ void tTimeWithMicroSecond() {
122124
new TimeString("14:22:47.123456"));
123125

124126
bitest(
125-
ExpressionCreator.time(false, (14L * 60 * 60 + 22 * 60 + 47) * 1000 * 1000 + 123456),
127+
ExpressionCreator.precisionTime(
128+
false, (14L * 60 * 60 + 22 * 60 + 47) * 1000 * 1000 + 123456, 6),
126129
rex.makeTimeLiteral(new TimeString("14:22:47.123456"), 6));
127130
}
128131

@@ -142,15 +145,23 @@ void tDate() {
142145

143146
@Test
144147
void tTimestamp() {
145-
TimestampLiteral ts = ExpressionCreator.timestamp(false, 2002, 2, 14, 16, 20, 47, 123);
148+
long epochMicro =
149+
TimeUnit.SECONDS.toMicros(
150+
LocalDateTime.of(2002, 2, 14, 16, 20, 47).toEpochSecond(ZoneOffset.UTC))
151+
+ 123;
152+
PrecisionTimestampLiteral ts = ExpressionCreator.precisionTimestamp(false, epochMicro, 6);
146153
int nano = (int) TimeUnit.MICROSECONDS.toNanos(123);
147154
TimestampString tsx = new TimestampString(2002, 2, 14, 16, 20, 47).withNanos(nano);
148155
bitest(ts, rex.makeTimestampLiteral(tsx, 6));
149156
}
150157

151158
@Test
152-
void tTimestampWithMilliMacroSeconds() {
153-
TimestampLiteral ts = ExpressionCreator.timestamp(false, 2002, 2, 14, 16, 20, 47, 123456);
159+
void tTimestampWithMilliMicroSeconds() {
160+
long epochMicro =
161+
TimeUnit.SECONDS.toMicros(
162+
LocalDateTime.of(2002, 2, 14, 16, 20, 47).toEpochSecond(ZoneOffset.UTC))
163+
+ 123456;
164+
PrecisionTimestampLiteral ts = ExpressionCreator.precisionTimestamp(false, epochMicro, 6);
154165
int nano = (int) TimeUnit.MICROSECONDS.toNanos(123456);
155166
TimestampString tsx = new TimestampString(2002, 2, 14, 16, 20, 47).withNanos(nano);
156167
bitest(ts, rex.makeTimestampLiteral(tsx, 6));

‎isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ void date(boolean nullable) {
104104
@ParameterizedTest
105105
@ValueSource(booleans = {true, false})
106106
void time(boolean nullable) {
107-
testType(Type.withNullability(nullable).TIME, SqlTypeName.TIME, nullable, 6);
107+
testType(Type.withNullability(nullable).precisionTime(6), SqlTypeName.TIME, nullable, 6);
108108
}
109109

110110
@ParameterizedTest

‎isthmus/src/test/java/io/substrait/isthmus/ExpressionConvertabilityTest.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,6 @@ void assertPrecisionTimestampLiteral(int precision) {
154154
void supportedPrecisionForPrecisionTimestampTZLiteral() {
155155
assertPrecisionTimestampTZLiteral(0);
156156
assertPrecisionTimestampTZLiteral(3);
157-
assertPrecisionTimestampTZLiteral(6);
158157
}
159158

160159
void assertPrecisionTimestampTZLiteral(int precision) {
@@ -207,7 +206,7 @@ void unsupportedPrecisionPrecisionTimestampTZLiteral() {
207206

208207
assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(4);
209208
assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(5);
210-
209+
assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(6);
211210
assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(7);
212211
assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(8);
213212

@@ -231,6 +230,6 @@ void assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(int precision)
231230

232231
void assertThrowsExpressionLiteral(Expression.Literal expr) {
233232
assertThrows(
234-
UnsupportedOperationException.class, () -> expr.accept(converter, Context.newContext()));
233+
IllegalArgumentException.class, () -> expr.accept(converter, Context.newContext()));
235234
}
236235
}

‎isthmus/src/test/java/io/substrait/isthmus/FunctionConversionTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ void extractPrecisionTimestampTzScalarFunction() {
103103
"extract:req_ptstz_str",
104104
TypeCreator.REQUIRED.I64,
105105
EnumArg.builder().value("MONTH").build(),
106-
Expression.PrecisionTimestampTZLiteral.builder().value(0).precision(6).build(),
106+
Expression.PrecisionTimestampTZLiteral.builder().value(0).precision(3).build(),
107107
Expression.StrLiteral.builder().value("GMT").build());
108108

109109
RexNode calciteExpr = reqPtstzFn.accept(expressionRexConverter, Context.newContext());
@@ -112,7 +112,7 @@ void extractPrecisionTimestampTzScalarFunction() {
112112

113113
RexCall extract = (RexCall) calciteExpr;
114114
assertEquals(
115-
"EXTRACT(FLAG(MONTH), 1970-01-01 00:00:00:TIMESTAMP_WITH_LOCAL_TIME_ZONE(6), 'GMT':VARCHAR)",
115+
"EXTRACT(FLAG(MONTH), 1970-01-01 00:00:00:TIMESTAMP_WITH_LOCAL_TIME_ZONE(3), 'GMT':VARCHAR)",
116116
extract.toString());
117117
}
118118

@@ -300,7 +300,7 @@ private static Stream<Arguments> strptimeTestCases() {
300300
"strptime_time:str_str",
301301
"12:34:56",
302302
"%H:%M:%S",
303-
TypeCreator.REQUIRED.TIME,
303+
TypeCreator.REQUIRED.precisionTime(6),
304304
"PARSE_TIME"),
305305
Arguments.of(
306306
"strptime_timestamp:str_str",

0 commit comments

Comments
 (0)