mihaibudiu commented on code in PR #5097:
URL: https://github.com/apache/calcite/pull/5097#discussion_r3573153358


##########
core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java:
##########
@@ -92,6 +94,34 @@ public MssqlSqlDialect(Context context) {
     top = context.databaseMajorVersion() < 11;
   }
 
+  @Override public @Nullable SqlNode getCastSpec(RelDataType type) {
+    switch (type.getSqlTypeName()) {
+    case TIMESTAMP:
+      // In SQL Server, TIMESTAMP is a deprecated synonym for ROWVERSION
+      // (a binary, auto-generated type), not a temporal type. The correct
+      // fixed-precision date/time type is DATETIME2 (SQL Server 2008+).
+      return createDatetimeCastSpec("DATETIME2", type);
+    case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
+      // SQL Server's timezone-aware date/time type.
+      return createDatetimeCastSpec("DATETIMEOFFSET", type);
+    default:
+      return super.getCastSpec(type);
+    }
+  }
+
+  /** Builds a SQL Server date/time cast target such as {@code DATETIME2(3)},
+   * appending the fractional-seconds precision when it is in the valid SQL
+   * Server range [0, 7]. */
+  private static SqlNode createDatetimeCastSpec(String typeAlias, RelDataType 
type) {
+    final int precision = type.getPrecision();
+    final String spec = precision >= 0 && precision <= 7
+        ? typeAlias + "(" + precision + ")"
+        : typeAlias;

Review Comment:
   So is a type without precision ok?
   Are the precision limits the same for DATETIME2 and DATETIMEOFFSET?



##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -1617,6 +1617,18 @@ private static String toSql(RelNode root, SqlDialect 
dialect,
     sql(query).ok(expected);
   }
 
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-7652";>[CALCITE-7652]
+   * MssqlSqlDialect unparses CAST to TIMESTAMP as "TIMESTAMP", which is 
invalid
+   * in SQL Server (should be DATETIME2)</a>. */
+  @Test void testCastToTimestampMssql() {
+    final String query = "select cast(\"hire_date\" as timestamp(3))\n"

Review Comment:
   There is no test for DATETIMEOFFSET (and it's not in the Jira either)



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to