You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/04/06 20:56:51 UTC
[GitHub] [druid] gianm opened a new pull request, #12407: SQL: Create millisecond precision timestamp literals.
gianm opened a new pull request, #12407:
URL: https://github.com/apache/druid/pull/12407
Fixes a bug where implicit casts of strings to timestamps would use seconds
precision rather than milliseconds. The new test case
testCountStarWithBetweenTimeFilterUsingMillisecondsInStringLiterals
exercises this.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 commented on pull request #12407: SQL: Create millisecond precision timestamp literals.
Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on PR #12407:
URL: https://github.com/apache/druid/pull/12407#issuecomment-1105944384
@gianm There're some failed cases and looks like they're related to the changes in this PR.
```bash
[ERROR] org.apache.druid.sql.calcite.CalciteSelectQueryTest.testSelectCurrentTimeAndDateLosAngeles Time elapsed: 0.064 s <<< FAILURE!
java.lang.AssertionError:
Conversion to relational algebra failed to preserve datatypes:
validated type:
RecordType(TIMESTAMP(0) NOT NULL CURRENT_TIMESTAMP, DATE NOT NULL CURRENT_DATE, DATE NOT NULL EXPR$2) NOT NULL
converted type:
RecordType(TIMESTAMP(3) NOT NULL CURRENT_TIMESTAMP, DATE NOT NULL CURRENT_DATE, DATE NOT NULL EXPR$2) NOT NULL
rel:
LogicalProject(CURRENT_TIMESTAMP=[1999-12-31 16:00:00:TIMESTAMP(3)], CURRENT_DATE=[1999-12-31], EXPR$2=[+(1999-12-31, 86400000:INTERVAL DAY)])
LogicalValues(tuples=[[{ 0 }]])
at org.apache.calcite.sql2rel.SqlToRelConverter.checkConvertedType(SqlToRelConverter.java:458)
at org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:575)
at org.apache.calcite.prepare.PlannerImpl.rel(PlannerImpl.java:254)
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm closed pull request #12407: SQL: Create millisecond precision timestamp literals.
Posted by GitBox <gi...@apache.org>.
gianm closed pull request #12407: SQL: Create millisecond precision timestamp literals.
URL: https://github.com/apache/druid/pull/12407
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm closed pull request #12407: SQL: Create millisecond precision timestamp literals.
Posted by GitBox <gi...@apache.org>.
gianm closed pull request #12407: SQL: Create millisecond precision timestamp literals.
URL: https://github.com/apache/druid/pull/12407
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 commented on a diff in pull request #12407: SQL: Create millisecond precision timestamp literals.
Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on code in PR #12407:
URL: https://github.com/apache/druid/pull/12407#discussion_r855053032
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java:
##########
@@ -82,12 +83,18 @@
private static final DateTimeFormatter CALCITE_TIME_PRINTER = DateTimeFormat.forPattern("HH:mm:ss.S");
private static final DateTimeFormatter CALCITE_DATE_PRINTER = DateTimeFormat.forPattern("yyyy-MM-dd");
- private static final DateTimeFormatter CALCITE_TIMESTAMP_PRINTER = DateTimeFormat.forPattern("yyyy-MM-dd HH:mm:ss.S");
+ private static final DateTimeFormatter CALCITE_TIMESTAMP_PRINTER =
+ DateTimeFormat.forPattern("yyyy-MM-dd HH:mm:ss.SSS");
private static final Charset DEFAULT_CHARSET = Charset.forName(ConversionUtil.NATIVE_UTF16_CHARSET_NAME);
private static final Pattern TRAILING_ZEROS = Pattern.compile("\\.?0+$");
+ /**
+ * Milliseconds precision.
+ */
+ private final static int TIMESTAMP_LITERAL_PRECISION = 3;
Review Comment:
```suggestion
private static final int TIMESTAMP_LITERAL_PRECISION = 3;
```
This fixes the checkstyle checking.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on pull request #12407: SQL: Create millisecond precision timestamp literals.
Posted by GitBox <gi...@apache.org>.
gianm commented on PR #12407:
URL: https://github.com/apache/druid/pull/12407#issuecomment-1106749390
> @gianm There're some failed cases and looks like they're related to the changes in this PR.
Ah, thanks. I looked into these, and there were some places I missed when setting the default precision to millis. I think the latest patch gets them all. I also added some additional tests to make sure.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a diff in pull request #12407: SQL: Create millisecond precision timestamp literals.
Posted by GitBox <gi...@apache.org>.
gianm commented on code in PR #12407:
URL: https://github.com/apache/druid/pull/12407#discussion_r855543126
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java:
##########
@@ -82,12 +83,18 @@
private static final DateTimeFormatter CALCITE_TIME_PRINTER = DateTimeFormat.forPattern("HH:mm:ss.S");
private static final DateTimeFormatter CALCITE_DATE_PRINTER = DateTimeFormat.forPattern("yyyy-MM-dd");
- private static final DateTimeFormatter CALCITE_TIMESTAMP_PRINTER = DateTimeFormat.forPattern("yyyy-MM-dd HH:mm:ss.S");
+ private static final DateTimeFormatter CALCITE_TIMESTAMP_PRINTER =
+ DateTimeFormat.forPattern("yyyy-MM-dd HH:mm:ss.SSS");
private static final Charset DEFAULT_CHARSET = Charset.forName(ConversionUtil.NATIVE_UTF16_CHARSET_NAME);
private static final Pattern TRAILING_ZEROS = Pattern.compile("\\.?0+$");
+ /**
+ * Milliseconds precision.
+ */
+ private final static int TIMESTAMP_LITERAL_PRECISION = 3;
Review Comment:
Thanks!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm merged pull request #12407: SQL: Create millisecond precision timestamp literals.
Posted by GitBox <gi...@apache.org>.
gianm merged PR #12407:
URL: https://github.com/apache/druid/pull/12407
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org