You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "tanclary (via GitHub)" <gi...@apache.org> on 2024/01/17 00:12:26 UTC
[PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
tanclary opened a new pull request, #3631:
URL: https://github.com/apache/calcite/pull/3631
…brary)
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3631:
URL: https://github.com/apache/calcite/pull/3631#discussion_r1457869015
##########
core/src/main/java/org/apache/calcite/sql/SqlKind.java:
##########
@@ -457,6 +457,9 @@ public enum SqlKind {
/** {@code TIME_SUB} function (BigQuery). */
TIME_SUB,
+ /** {@code TIMESTAMP} function (BigQuery). */
+ TIMESTAMP,
Review Comment:
Done, didn't need to add that anyways I don't think.
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1610,6 +1610,36 @@ private static RelDataType deriveTypeMapFromEntries(SqlOperatorBinding opBinding
OperandTypes.STRING_STRING,
SqlFunctionCategory.TIMEDATE);
+ /** The "TO_TIMESTAMP_LTZ" function returns a Calcite
+ * {@code TIMESTAMP WITH LOCAL TIME ZONE}.
+ * It has the following overloads (Snowflake also
+ * supports a variant and quoted integer overload but
+ * those are not yet supported):
+ *
+ * <ul>
+ * <li>{@code TO_TIMESTAMP_LTZ(numeric[, scale)]}
+ * <li>{@code TO_TIMESTAMP_LTZ(date)}
+ * <li>{@code TO_TIMESTAMP_LTZ(datetime)}
Review Comment:
Done
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
Posted by "JiajunBernoulli (via GitHub)" <gi...@apache.org>.
JiajunBernoulli commented on PR #3631:
URL: https://github.com/apache/calcite/pull/3631#issuecomment-1894805047
Does CI stability failed?
https://github.com/apache/calcite/actions/runs/7549299627/attempts/1?pr=3631
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3631:
URL: https://github.com/apache/calcite/pull/3631#issuecomment-1899100154
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3631) **Quality Gate passed**
The SonarCloud Quality Gate passed, but some issues were introduced.
[4 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3631&resolved=false&inNewCodePeriod=true)
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3631&resolved=false&inNewCodePeriod=true)
[100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3631&metric=new_coverage&view=list)
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3631&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3631)
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3631:
URL: https://github.com/apache/calcite/pull/3631#discussion_r1457828057
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -2480,6 +2484,32 @@ private static class RegexpReplaceImplementor extends AbstractRexCallImplementor
}
}
+ /** Implementor for the {@code TO_TIMESTAMP_LTZ} function. */
Review Comment:
The main reason I added the implementor was to avoid the "error proneness" you mentioned in your above comment. It checks the type of the first argument (i.e. `DATE`, `TIMESTAMP`, etc.) to ensure it does not call the wrong method via reflection.
Before I added the implementor, when I was trying to use `TO_TIMESTAMP_LTZ(numeric)`, it would end up using the method for `DATE` arguments since dates are represented as integers. This way, that confusion is avoided.
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3631:
URL: https://github.com/apache/calcite/pull/3631#discussion_r1457825819
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4461,6 +4461,51 @@ public static int time(long timestampMillis, String timeZone) {
/ (1000L * 1000L)); // milli > micro > nano
}
+ /** SQL {@code TO_TIMESTAMP_LTZ(date)} function
+ * for a date values. */
+ public static long toTimestampLtzDate(int days) {
+ return timestamp(days);
Review Comment:
Yeah I just was not sure how else to handle the fact that you can provide the function a `DATE` which gets converted to an `int` or you can provide the function with a numeric value (which could be a long).
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #3631:
URL: https://github.com/apache/calcite/pull/3631#issuecomment-1896331624
Seems that the "Error Prone" check is unhappy.
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3631:
URL: https://github.com/apache/calcite/pull/3631#discussion_r1457824410
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4461,6 +4461,51 @@ public static int time(long timestampMillis, String timeZone) {
/ (1000L * 1000L)); // milli > micro > nano
}
+ /** SQL {@code TO_TIMESTAMP_LTZ(date)} function
+ * for a date values. */
+ public static long toTimestampLtzDate(int days) {
+ return timestamp(days);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds)}
+ * function for long values. */
+ public static long toTimestampLtz(long timestampSeconds) {
+ return toTimestampLtz(timestampSeconds, 0);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds)} function
+ * for BigDecimal values. */
+ public static long toTimestampLtz(BigDecimal timestampSeconds) {
+ return toTimestampLtz(timestampSeconds.longValue(), 0);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds, scale)}
+ * function for BigDecimal values with a specified scale. */
+ public static long toTimestampLtz(BigDecimal timestampSeconds, int scale) {
+ return toTimestampLtz(timestampSeconds.longValue(), scale);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds, scale)}
+ * function for long values with a specified scale. */
+ public static long toTimestampLtz(long timestampSeconds, int scale) {
+ // 0 means seconds but we receive milliseconds so must adjust
+ final int trueScale = 3 - scale;
+ final double multiplier = Math.pow(10, trueScale);
+ return (long) (timestampSeconds * multiplier);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds, scale)}
+ * function for a String expression of a timestamp. */
+ public static long toTimestampLtz(String timestamp) {
+ return timestamp(timestamp);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestamp)} function
Review Comment:
I was under the impression a Calcite `TIMESTAMP` did not have a LTZ but `TIMESTAMP WITH LOCAL TIME ZONE` did. So if you provide the function a Calcite `TIMESTAMP` it would return a `TIMESTAMP WITH LOCAL TIME ZONE` (not a no-op).
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3631:
URL: https://github.com/apache/calcite/pull/3631#discussion_r1486012354
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -8747,6 +8747,55 @@ private void testCurrentDateFunc(Pair<String, Hook.Closeable> pair) {
}
}
+ /** Tests the {@code TO_TIMESTAMP_LTZ} operator. */
+ @Test void testToTimestampLtzFunc() {
+ final SqlOperatorFixture f0 = fixture();
+ f0.setFor(SqlLibraryOperators.TO_TIMESTAMP_LTZ);
+ f0.checkFails("^TO_TIMESTAMP_LTZ(1)^",
+ "No match found for function signature "
+ + "TO_TIMESTAMP_LTZ\\(<NUMERIC>\\)", false);
+ final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SNOWFLAKE);
+ // TO_TIMESTAMP_LTZ(numeric)
+ f.checkScalar("to_timestamp_ltz(0)",
+ "1970-01-01 00:00:00",
+ "TIMESTAMP_WITH_LOCAL_TIME_ZONE(0) NOT NULL");
+ f.checkScalar("to_timestamp_ltz(123456)",
+ "1970-01-02 10:17:36",
+ "TIMESTAMP_WITH_LOCAL_TIME_ZONE(0) NOT NULL");
+ f.checkScalar("to_timestamp_ltz(123456.78)",
+ "1970-01-02 10:17:36",
+ "TIMESTAMP_WITH_LOCAL_TIME_ZONE(0) NOT NULL");
+ f.checkNull("to_timestamp_ltz(null)");
+ // TO_TIMESTAMP_LTZ(numeric, scale)
+ f.checkScalar("to_timestamp_ltz(123456, 3)",
+ "1970-01-01 00:02:03",
+ "TIMESTAMP_WITH_LOCAL_TIME_ZONE(0) NOT NULL");
+ f.checkScalar("to_timestamp_ltz(123456891234343434, 9)",
+ "1973-11-29 21:34:51",
+ "TIMESTAMP_WITH_LOCAL_TIME_ZONE(0) NOT NULL");
+ f.checkScalar("to_timestamp_ltz(12345689.123, 2)",
+ "1970-01-02 10:17:36",
+ "TIMESTAMP_WITH_LOCAL_TIME_ZONE(0) NOT NULL");
+ f.checkNull("to_timestamp_ltz(null, 2)");
Review Comment:
Hello，to_timestamp_ltz(null, null) will report an error?
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3631:
URL: https://github.com/apache/calcite/pull/3631#discussion_r1457870070
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1610,6 +1610,36 @@ private static RelDataType deriveTypeMapFromEntries(SqlOperatorBinding opBinding
OperandTypes.STRING_STRING,
SqlFunctionCategory.TIMEDATE);
+ /** The "TO_TIMESTAMP_LTZ" function returns a Calcite
Review Comment:
Done
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3631:
URL: https://github.com/apache/calcite/pull/3631#discussion_r1457830260
##########
site/_docs/reference.md:
##########
@@ -2746,7 +2746,7 @@ In the following:
| h s | FORMAT_NUMBER(value, decimalVal) | Formats the number *value* like '#,###,###.##', rounded to decimal places *decimalVal*. If *decimalVal* is 0, the result has no decimal point or fractional part
| h s | FORMAT_NUMBER(value, format) | Formats the number *value* to MySQL's FORMAT *format*, like '#,###,###.##0.00'
| b | FORMAT_TIME(string, time) | Formats *time* according to the specified format *string*
-| b | FORMAT_TIMESTAMP(string timestamp) | Formats *timestamp* according to the specified format *string*
+| b | FORMAT_TIMESTAMP(string, timestamp) | Formats *timestamp* according to the specified format *string*
Review Comment:
Done
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3631:
URL: https://github.com/apache/calcite/pull/3631#discussion_r1457868807
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4461,6 +4461,51 @@ public static int time(long timestampMillis, String timeZone) {
/ (1000L * 1000L)); // milli > micro > nano
}
+ /** SQL {@code TO_TIMESTAMP_LTZ(date)} function
+ * for a date values. */
+ public static long toTimestampLtzDate(int days) {
+ return timestamp(days);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds)}
+ * function for long values. */
+ public static long toTimestampLtz(long timestampSeconds) {
+ return toTimestampLtz(timestampSeconds, 0);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds)} function
+ * for BigDecimal values. */
+ public static long toTimestampLtz(BigDecimal timestampSeconds) {
+ return toTimestampLtz(timestampSeconds.longValue(), 0);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds, scale)}
+ * function for BigDecimal values with a specified scale. */
+ public static long toTimestampLtz(BigDecimal timestampSeconds, int scale) {
+ return toTimestampLtz(timestampSeconds.longValue(), scale);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds, scale)}
+ * function for long values with a specified scale. */
Review Comment:
Added a longer comment that explains the `scale` parameter better. Added an example too. Let me know what you think.
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3631:
URL: https://github.com/apache/calcite/pull/3631#discussion_r1456238725
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1610,6 +1610,36 @@ private static RelDataType deriveTypeMapFromEntries(SqlOperatorBinding opBinding
OperandTypes.STRING_STRING,
SqlFunctionCategory.TIMEDATE);
+ /** The "TO_TIMESTAMP_LTZ" function returns a Calcite
Review Comment:
It's worth spelling out that a `TIMESTAMP WITH LOCAL TIME ZONE` is stored as millis since UTC epoch.
Describe which time zone is used for conversion from `DATE` and `TIMESTAMP`.
##########
core/src/main/java/org/apache/calcite/sql/SqlKind.java:
##########
@@ -457,6 +457,9 @@ public enum SqlKind {
/** {@code TIME_SUB} function (BigQuery). */
TIME_SUB,
+ /** {@code TIMESTAMP} function (BigQuery). */
+ TIMESTAMP,
Review Comment:
not a good name, since TIMESTAMP is a type
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1610,6 +1610,36 @@ private static RelDataType deriveTypeMapFromEntries(SqlOperatorBinding opBinding
OperandTypes.STRING_STRING,
SqlFunctionCategory.TIMEDATE);
+ /** The "TO_TIMESTAMP_LTZ" function returns a Calcite
+ * {@code TIMESTAMP WITH LOCAL TIME ZONE}.
+ * It has the following overloads (Snowflake also
+ * supports a variant and quoted integer overload but
+ * those are not yet supported):
+ *
+ * <ul>
+ * <li>{@code TO_TIMESTAMP_LTZ(numeric[, scale)]}
+ * <li>{@code TO_TIMESTAMP_LTZ(date)}
+ * <li>{@code TO_TIMESTAMP_LTZ(datetime)}
Review Comment:
datetime is not a Calcite type
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4461,6 +4461,51 @@ public static int time(long timestampMillis, String timeZone) {
/ (1000L * 1000L)); // milli > micro > nano
}
+ /** SQL {@code TO_TIMESTAMP_LTZ(date)} function
+ * for a date values. */
+ public static long toTimestampLtzDate(int days) {
+ return timestamp(days);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds)}
+ * function for long values. */
+ public static long toTimestampLtz(long timestampSeconds) {
+ return toTimestampLtz(timestampSeconds, 0);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds)} function
+ * for BigDecimal values. */
+ public static long toTimestampLtz(BigDecimal timestampSeconds) {
+ return toTimestampLtz(timestampSeconds.longValue(), 0);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds, scale)}
+ * function for BigDecimal values with a specified scale. */
+ public static long toTimestampLtz(BigDecimal timestampSeconds, int scale) {
+ return toTimestampLtz(timestampSeconds.longValue(), scale);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds, scale)}
+ * function for long values with a specified scale. */
Review Comment:
Can you add examples? Does `toTimestampLtz(86_400L, 0)` mean `TIMESTAMP '1970-01-02 00:00:00'`?
What would `toTimestampLtz(86_400L, 3)` mean?
##########
site/_docs/reference.md:
##########
@@ -2746,7 +2746,7 @@ In the following:
| h s | FORMAT_NUMBER(value, decimalVal) | Formats the number *value* like '#,###,###.##', rounded to decimal places *decimalVal*. If *decimalVal* is 0, the result has no decimal point or fractional part
| h s | FORMAT_NUMBER(value, format) | Formats the number *value* to MySQL's FORMAT *format*, like '#,###,###.##0.00'
| b | FORMAT_TIME(string, time) | Formats *time* according to the specified format *string*
-| b | FORMAT_TIMESTAMP(string timestamp) | Formats *timestamp* according to the specified format *string*
+| b | FORMAT_TIMESTAMP(string, timestamp) | Formats *timestamp* according to the specified format *string*
Review Comment:
remove space before bar
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4461,6 +4461,51 @@ public static int time(long timestampMillis, String timeZone) {
/ (1000L * 1000L)); // milli > micro > nano
}
+ /** SQL {@code TO_TIMESTAMP_LTZ(date)} function
+ * for a date values. */
+ public static long toTimestampLtzDate(int days) {
+ return timestamp(days);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds)}
+ * function for long values. */
+ public static long toTimestampLtz(long timestampSeconds) {
+ return toTimestampLtz(timestampSeconds, 0);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds)} function
+ * for BigDecimal values. */
+ public static long toTimestampLtz(BigDecimal timestampSeconds) {
+ return toTimestampLtz(timestampSeconds.longValue(), 0);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds, scale)}
+ * function for BigDecimal values with a specified scale. */
+ public static long toTimestampLtz(BigDecimal timestampSeconds, int scale) {
+ return toTimestampLtz(timestampSeconds.longValue(), scale);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds, scale)}
+ * function for long values with a specified scale. */
+ public static long toTimestampLtz(long timestampSeconds, int scale) {
+ // 0 means seconds but we receive milliseconds so must adjust
+ final int trueScale = 3 - scale;
+ final double multiplier = Math.pow(10, trueScale);
+ return (long) (timestampSeconds * multiplier);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestampSeconds, scale)}
+ * function for a String expression of a timestamp. */
+ public static long toTimestampLtz(String timestamp) {
+ return timestamp(timestamp);
+ }
+
+ /** SQL {@code TO_TIMESTAMP_LTZ(timestamp)} function
Review Comment:
be explicit what the argument means. millis since local epoch? millis since UTC epoch?
using 'timestamp' in the function name would by default mean a Calcite timestamp, i.e. ltz. But then the function would be a no-op.
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -2480,6 +2484,32 @@ private static class RegexpReplaceImplementor extends AbstractRexCallImplementor
}
}
+ /** Implementor for the {@code TO_TIMESTAMP_LTZ} function. */
Review Comment:
is an implementor necessary? it would be better if calls to TO_TIMESTAMP_LTZ were translated to existing functions. Then we would not need new methods in `SqlFunctions`
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4461,6 +4461,51 @@ public static int time(long timestampMillis, String timeZone) {
/ (1000L * 1000L)); // milli > micro > nano
}
+ /** SQL {@code TO_TIMESTAMP_LTZ(date)} function
+ * for a date values. */
+ public static long toTimestampLtzDate(int days) {
+ return timestamp(days);
Review Comment:
having overloaded `timestamp(int)` and `timestamp(long)` functions with different semantics is error-prone
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3631:
URL: https://github.com/apache/calcite/pull/3631#issuecomment-1899554121
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3631) **Quality Gate passed**
The SonarCloud Quality Gate passed, but some issues were introduced.
[4 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3631&resolved=false&inNewCodePeriod=true)
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3631&resolved=false&inNewCodePeriod=true)
[100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3631&metric=new_coverage&view=list)
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3631&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3631)
--
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@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org