(calcite-avatica) branch main updated: [CALCITE-6212] Config locale = en_US for javadoc task
This is an automated email from the ASF dual-hosted git repository.
libenchao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite-avatica.git
The following commit(s) were added to refs/heads/main by this push:
new 07c6b8d92 [CALCITE-6212] Config locale = en_US for javadoc task
07c6b8d92 is described below
commit 07c6b8d92f478646e3e605a80dc0d60329c37d33
Author: Benchao Li
AuthorDate: Fri Jan 19 10:37:42 2024 +0800
[CALCITE-6212] Config locale = en_US for javadoc task
---
build.gradle.kts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/build.gradle.kts b/build.gradle.kts
index b866f0e8f..465f194a1 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -241,6 +241,8 @@ allprojects {
(options as StandardJavadocDocletOptions).apply {
// Please refrain from using non-ASCII chars below since the
options are passed as
// javadoc.options file which is parsed with "default encoding"
+// locale should be placed at the head of any options:
https://docs.gradle.org/current/javadoc/org/gradle/external/javadoc/CoreJavadocOptions.html#getLocale
+locale = "en_US"
noTimestamp.value = true
showFromProtected()
// javadoc: error - The code being documented uses modules but
the packages
Re: [PR] [CALCITE-6212] Config locale = en_US for javadoc task [calcite-avatica]
libenchao merged PR #235: URL: https://github.com/apache/calcite-avatica/pull/235 -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6111] Explicit cast from expression to numeric type doesn't check overflow [calcite]
asolimando commented on code in PR #3522:
URL: https://github.com/apache/calcite/pull/3522#discussion_r1458498638
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -643,11 +647,12 @@ void testCastExactNumericLimits(CastType castType,
SqlOperatorFixture f) {
f.checkCastToScalarOkay("'" + numeric.minNumericString + "'",
type, numeric.minNumericString, castType);
- if (Bug.CALCITE_2539_FIXED) {
+ if (numeric != Numeric.DECIMAL5_2) {
+// The above condition is for bug CALCITE-6078
f.checkCastFails("'" + numeric.maxOverflowNumericString + "'",
-type, OUT_OF_RANGE_MESSAGE, true, castType);
+type, WRONG_FORMAT_MESSAGE, true, castType);
Review Comment:
You are right, I missed the extra quote, sorry about that!
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
sonarcloud[bot] commented on PR #3631: URL: https://github.com/apache/calcite/pull/3631#issuecomment-1899554121 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=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=3631=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3631=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3631=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3631=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6212] Config locale = 'en_US' for javadoc task [calcite]
libenchao commented on code in PR #3637:
URL: https://github.com/apache/calcite/pull/3637#discussion_r1458215905
##
build.gradle.kts:
##
@@ -486,6 +486,8 @@ allprojects {
(options as StandardJavadocDocletOptions).apply {
// Please refrain from using non-ASCII chars below since the
options are passed as
// javadoc.options file which is parsed with "default encoding"
+// locale should be placed at the head of any options:
https://docs.gradle.org/current/javadoc/org/gradle/external/javadoc/CoreJavadocOptions.html#getLocale
+locale = "en_US"
Review Comment:
The PR against avatica is now available for review:
https://github.com/apache/calcite-avatica/pull/235
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] [CALCITE-6212] Config locale = en_US for javadoc task [calcite-avatica]
libenchao opened a new pull request, #235: URL: https://github.com/apache/calcite-avatica/pull/235 (no comment) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(calcite) branch main updated: [CALCITE-6212] Config locale = 'en_US' for javadoc task
This is an automated email from the ASF dual-hosted git repository.
libenchao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new d5fa3eb074 [CALCITE-6212] Config locale = 'en_US' for javadoc task
d5fa3eb074 is described below
commit d5fa3eb07492675e08308635967117932f98478f
Author: Benchao Li
AuthorDate: Thu Jan 18 22:49:13 2024 +0800
[CALCITE-6212] Config locale = 'en_US' for javadoc task
Close apache/calcite#3637
---
build.gradle.kts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/build.gradle.kts b/build.gradle.kts
index dc6a91658e..0df08ac15c 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -486,6 +486,8 @@ allprojects {
(options as StandardJavadocDocletOptions).apply {
// Please refrain from using non-ASCII chars below since the
options are passed as
// javadoc.options file which is parsed with "default encoding"
+// locale should be placed at the head of any options:
https://docs.gradle.org/current/javadoc/org/gradle/external/javadoc/CoreJavadocOptions.html#getLocale
+locale = "en_US"
noTimestamp.value = true
showFromProtected()
// javadoc: error - The code being documented uses modules but
the packages
Re: [PR] [CALCITE-6212] Config locale = 'en_US' for javadoc task [calcite]
libenchao closed pull request #3637: [CALCITE-6212] Config locale = 'en_US' for javadoc task URL: https://github.com/apache/calcite/pull/3637 -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6212] Config locale = 'en_US' for javadoc task [calcite]
libenchao commented on code in PR #3637:
URL: https://github.com/apache/calcite/pull/3637#discussion_r1458197251
##
build.gradle.kts:
##
@@ -486,6 +486,8 @@ allprojects {
(options as StandardJavadocDocletOptions).apply {
// Please refrain from using non-ASCII chars below since the
options are passed as
// javadoc.options file which is parsed with "default encoding"
+// locale should be placed at the head of any options:
https://docs.gradle.org/current/javadoc/org/gradle/external/javadoc/CoreJavadocOptions.html#getLocale
+locale = "en_US"
Review Comment:
Yes, I do think so. I'll do this for avatica once this is merged.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6212] Config locale = 'en_US' for javadoc task [calcite]
libenchao commented on PR #3637: URL: https://github.com/apache/calcite/pull/3637#issuecomment-1899532571 > Is it better to register the affected version with Jira? I've logged CALCITE-6212, 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6211] SUBSTRING with Integer.MIN_VALUE as a second parameter raise unexpected exception [calcite]
JiajunBernoulli commented on PR #3636: URL: https://github.com/apache/calcite/pull/3636#issuecomment-1899512259 Here is same issue: https://github.com/apache/calcite/blob/3d66b346ff2dcf23b58a9fcb144f40600cce7b2c/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java#L1041 Can you fix it in the PR? -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Build: config locale = 'en_US' for javadoc task [calcite]
JiajunBernoulli commented on PR #3637: URL: https://github.com/apache/calcite/pull/3637#issuecomment-1899500249 Is it better to register the affected version with Jira? -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-5649] Get row count statistics from ReflectiveSchema [calcite]
jduo commented on PR #3634: URL: https://github.com/apache/calcite/pull/3634#issuecomment-1899425359 @mihaibudiu , were there any other concerns to address with this PR? -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CALCITE-5647 Use mq.getRowCount(rel) instead of rel.estimateRowCount(mq) [calcite]
jduo commented on PR #3632: URL: https://github.com/apache/calcite/pull/3632#issuecomment-1899425810 @JiajunBernoulli @mihaibudiu , were there any other concerns to address in this PR? -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CALCITE-6208 update JSON_VALUE return type inference to make explicit array return types be nullable with nullable elements [calcite]
clintropolis commented on code in PR #3633:
URL: https://github.com/apache/calcite/pull/3633#discussion_r1458069779
##
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##
@@ -11413,6 +11413,14 @@ private void checkCustomColumnResolving(String table) {
expr("json_value('{\"foo\":100}', 'lax $.foo' returning boolean"
+ " default 100 on empty)")
.columnType("BOOLEAN");
+
+expr("json_value('{\"foo\":[100, null, 200]}', 'lax $.foo'"
++ "returning integer array)")
+.columnType("INTEGER ARRAY");
+
+expr("json_value('{\"foo\":[[100, null, 200]]}', 'lax $.foo'"
Review Comment:
Yeah, I agree it would be nice to have, but don't really have any
pre-existing code for the calcite side of things for this. Druid could probably
make use of it if such a thing exists as we store this type of varying json
data in their original types in our segments, but right now at query time we
best effort convert them to a common type for query processing.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6111] Explicit cast from expression to numeric type doesn't check overflow [calcite]
mihaibudiu commented on code in PR #3522:
URL: https://github.com/apache/calcite/pull/3522#discussion_r1458056857
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -643,11 +647,12 @@ void testCastExactNumericLimits(CastType castType,
SqlOperatorFixture f) {
f.checkCastToScalarOkay("'" + numeric.minNumericString + "'",
type, numeric.minNumericString, castType);
- if (Bug.CALCITE_2539_FIXED) {
+ if (numeric != Numeric.DECIMAL5_2) {
+// The above condition is for bug CALCITE-6078
f.checkCastFails("'" + numeric.maxOverflowNumericString + "'",
-type, OUT_OF_RANGE_MESSAGE, true, castType);
+type, WRONG_FORMAT_MESSAGE, true, castType);
Review Comment:
The integer-to-integer cast is tested a few lines above.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6111] Explicit cast from expression to numeric type doesn't check overflow [calcite]
mihaibudiu commented on code in PR #3522:
URL: https://github.com/apache/calcite/pull/3522#discussion_r1458056660
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -643,11 +647,12 @@ void testCastExactNumericLimits(CastType castType,
SqlOperatorFixture f) {
f.checkCastToScalarOkay("'" + numeric.minNumericString + "'",
type, numeric.minNumericString, castType);
- if (Bug.CALCITE_2539_FIXED) {
+ if (numeric != Numeric.DECIMAL5_2) {
+// The above condition is for bug CALCITE-6078
f.checkCastFails("'" + numeric.maxOverflowNumericString + "'",
-type, OUT_OF_RANGE_MESSAGE, true, castType);
+type, WRONG_FORMAT_MESSAGE, true, castType);
Review Comment:
Actually this variant of the test invokes something like
`Integer.parseInt(string)`. So this is in fact a cast from a string to integer
-- notice the single quotes added around the literal -- which gives a different
error than a cast from a larger integer to an integer. The former produces a
WRONG_FORMAT, the latter gives OUT_OF_RANGE. We could in principle trap the
exception and rewrite the message, but it *is* a different exception in Java in
the simplification code.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CALCITE-6208 update JSON_VALUE return type inference to make explicit array return types be nullable with nullable elements [calcite]
clintropolis commented on code in PR #3633:
URL: https://github.com/apache/calcite/pull/3633#discussion_r1457992830
##
core/src/main/java/org/apache/calcite/sql/fun/SqlJsonValueFunction.java:
##
@@ -54,20 +53,33 @@ public class SqlJsonValueFunction extends SqlFunction {
public SqlJsonValueFunction(String name) {
super(name, SqlKind.OTHER_FUNCTION,
-ReturnTypes.cascade(
-opBinding ->
explicitTypeSpec(opBinding).orElse(getDefaultType(opBinding)),
-SqlTypeTransforms.FORCE_NULLABLE),
+opBinding -> explicitTypeSpec(opBinding)
+.map(relDataType -> deriveExplicitType(opBinding, relDataType))
+.orElseGet(() -> getDefaultType(opBinding)),
null,
OperandTypes.family(
ImmutableList.of(SqlTypeFamily.ANY, SqlTypeFamily.CHARACTER),
ordinal -> ordinal > 1),
SqlFunctionCategory.SYSTEM);
}
+ private static RelDataType deriveExplicitType(SqlOperatorBinding opBinding,
RelDataType type) {
+if (SqlTypeName.ARRAY == type.getSqlTypeName()) {
+ RelDataType elementType =
Objects.requireNonNull(type.getComponentType());
+ RelDataType nullableElementType = deriveExplicitType(opBinding,
elementType);
+ return SqlTypeUtil.createArrayType(
+ opBinding.getTypeFactory(),
+ nullableElementType,
+ true);
+}
+return opBinding.getTypeFactory().createTypeWithNullability(type, true);
+ }
+
/** Returns VARCHAR(2000) as default. */
private static RelDataType getDefaultType(SqlOperatorBinding opBinding) {
final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
-return typeFactory.createSqlType(SqlTypeName.VARCHAR, 2000);
+final RelDataType baseType =
typeFactory.createSqlType(SqlTypeName.VARCHAR, 2000);
Review Comment:
yeah, happy to change it in this PR, it seems like a strange default to me
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
sonarcloud[bot] commented on PR #3631: URL: https://github.com/apache/calcite/pull/3631#issuecomment-1899100154 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=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=3631=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3631=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3631=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3631=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
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):
+ *
+ *
+ * {@code TO_TIMESTAMP_LTZ(numeric[, scale)]}
+ * {@code TO_TIMESTAMP_LTZ(date)}
+ * {@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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6198] Add TO_TIMESTAMP_LTZ function (enabled in Snowflake li… [calcite]
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6098] Update LICENSE and NOTICE for Jekyll website template [calcite]
mihaibudiu commented on PR #3510: URL: https://github.com/apache/calcite/pull/3510#issuecomment-1898976217 Licensing and the way it should be surfaced in documentation is not my forte, but the changes look fine otherwise. -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CALCITE-6208 update JSON_VALUE return type inference to make explicit array return types be nullable with nullable elements [calcite]
mihaibudiu commented on code in PR #3633:
URL: https://github.com/apache/calcite/pull/3633#discussion_r1457813541
##
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##
@@ -11413,6 +11413,14 @@ private void checkCustomColumnResolving(String table) {
expr("json_value('{\"foo\":100}', 'lax $.foo' returning boolean"
+ " default 100 on empty)")
.columnType("BOOLEAN");
+
+expr("json_value('{\"foo\":[100, null, 200]}', 'lax $.foo'"
++ "returning integer array)")
+.columnType("INTEGER ARRAY");
+
+expr("json_value('{\"foo\":[[100, null, 200]]}', 'lax $.foo'"
Review Comment:
This has been a desired feature in Calcite for quite some time:
https://issues.apache.org/jira/browse/CALCITE-4918
Does anyone plan to contribute it (assuming you are using Calcite for this
purpose)?
I was thinking to do it myself, but if you already have the code and the
tests, it would be great.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CALCITE-6208 update JSON_VALUE return type inference to make explicit array return types be nullable with nullable elements [calcite]
mihaibudiu commented on code in PR #3633:
URL: https://github.com/apache/calcite/pull/3633#discussion_r1457811681
##
core/src/main/java/org/apache/calcite/sql/fun/SqlJsonValueFunction.java:
##
@@ -54,20 +53,33 @@ public class SqlJsonValueFunction extends SqlFunction {
public SqlJsonValueFunction(String name) {
super(name, SqlKind.OTHER_FUNCTION,
-ReturnTypes.cascade(
-opBinding ->
explicitTypeSpec(opBinding).orElse(getDefaultType(opBinding)),
-SqlTypeTransforms.FORCE_NULLABLE),
+opBinding -> explicitTypeSpec(opBinding)
+.map(relDataType -> deriveExplicitType(opBinding, relDataType))
+.orElseGet(() -> getDefaultType(opBinding)),
null,
OperandTypes.family(
ImmutableList.of(SqlTypeFamily.ANY, SqlTypeFamily.CHARACTER),
ordinal -> ordinal > 1),
SqlFunctionCategory.SYSTEM);
}
+ private static RelDataType deriveExplicitType(SqlOperatorBinding opBinding,
RelDataType type) {
+if (SqlTypeName.ARRAY == type.getSqlTypeName()) {
+ RelDataType elementType =
Objects.requireNonNull(type.getComponentType());
+ RelDataType nullableElementType = deriveExplicitType(opBinding,
elementType);
+ return SqlTypeUtil.createArrayType(
+ opBinding.getTypeFactory(),
+ nullableElementType,
+ true);
+}
+return opBinding.getTypeFactory().createTypeWithNullability(type, true);
+ }
+
/** Returns VARCHAR(2000) as default. */
private static RelDataType getDefaultType(SqlOperatorBinding opBinding) {
final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
-return typeFactory.createSqlType(SqlTypeName.VARCHAR, 2000);
+final RelDataType baseType =
typeFactory.createSqlType(SqlTypeName.VARCHAR, 2000);
Review Comment:
I have fixed a couple of bugs in the past related to that:
https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-5813. This very
much looks like a bug to me. It means that if you get a result longer than 2000
characters it will be truncated. And that's not what you want. Whether you want
to fix it in this PR or another one, that's a different issue.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6205] Add BITAND_AGG, BITOR_AGG functions (enabled in Snowfl… [calcite]
tanclary commented on code in PR #3630:
URL: https://github.com/apache/calcite/pull/3630#discussion_r1457806290
##
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##
@@ -6662,6 +6662,28 @@ private void checkLiteral2(String expression, String
expected) {
sql(sql).withMysql().ok(expectedMysql);
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6205;>[CALCITE-6205]
+ * Add BITAND_AGG, BITOR_AGG functions (enabled in Snowflake library). */
+ @Test void testBitAndAgg() {
+final String query = "select bit_and(\"product_id\")\n"
++ "from \"product\"";
+final String expectedSnowflake = "SELECT BITAND_AGG(\"product_id\")\n"
++ "FROM \"foodmart\".\"product\"";
+
sql(query).withLibrary(SqlLibrary.SNOWFLAKE).withSnowflake().ok(expectedSnowflake);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6205;>[CALCITE-6205]
+ * Add BITAND_AGG, BITOR_AGG functions (enabled in Snowflake library). */
+ @Test void testBitOrAgg() {
+final String query = "select bit_or(\"product_id\")\n"
++ "from \"product\"";
+final String expectedSnowflake = "SELECT BITOR_AGG(\"product_id\")\n"
Review Comment:
My JIRA case was actually wrong, `BIT_OR` and `BIT_AND` are both standard
operators not specific to BigQuery, I updated the case.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6205] Add BITAND_AGG, BITOR_AGG functions (enabled in Snowfl… [calcite]
tanclary commented on code in PR #3630:
URL: https://github.com/apache/calcite/pull/3630#discussion_r1457803121
##
core/src/main/java/org/apache/calcite/sql/fun/SqlBitOpAggFunction.java:
##
@@ -39,7 +39,7 @@ public class SqlBitOpAggFunction extends SqlAggFunction {
//~ Constructors ---
- /** Creates a SqlBitOpAggFunction. */
+ /** Creates a SqlBitOpAggFunction from a SqlKind. */
Review Comment:
Actually re-reading this I think I would prefer keeping the
`SqlBitOpAggFunction(SqlKind kind)` constructor, I think it makes sense to only
pass in one parameter if possible.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Build: config locale = 'en_US' for javadoc task [calcite]
snuyanzin commented on code in PR #3637:
URL: https://github.com/apache/calcite/pull/3637#discussion_r1457562119
##
build.gradle.kts:
##
@@ -486,6 +486,8 @@ allprojects {
(options as StandardJavadocDocletOptions).apply {
// Please refrain from using non-ASCII chars below since the
options are passed as
// javadoc.options file which is parsed with "default encoding"
+// locale should be placed at the head of any options:
https://docs.gradle.org/current/javadoc/org/gradle/external/javadoc/CoreJavadocOptions.html#getLocale
+locale = "en_US"
Review Comment:
Nice finding
Am I correct that same thing should be done for Avatica?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] Build: config locale = 'en_US' for javadoc task [calcite]
libenchao opened a new pull request, #3637: URL: https://github.com/apache/calcite/pull/3637 (no comment) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6098] Update LICENSE and NOTICE for Jekyll website template [calcite]
zabetak commented on PR #3510: URL: https://github.com/apache/calcite/pull/3510#issuecomment-1898301207 @mihaibudiu @XuQianJin-Stars After discussion under the JIRA I took quite a different approach in this PR (thus the rebase). Letting you know in case you want to reconsider your approval. -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6111] Explicit cast from expression to numeric type doesn't check overflow [calcite]
asolimando commented on code in PR #3522:
URL: https://github.com/apache/calcite/pull/3522#discussion_r1457093609
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -13695,6 +13698,43 @@ private static void
checkLogicalOrFunc(SqlOperatorFixture f) {
}
}
+ /**
+ * Test cases for
+ * https://issues.apache.org/jira/browse/CALCITE-6111;>[CALCITE-6111]
+ * Explicit cast from expression to numeric type doesn't check overflow.
*/
+ @Test public void testOverflow() {
+final SqlOperatorFixture f = fixture();
+f.checkFails(String.format(Locale.US, "SELECT cast(%d+30 as tinyint)",
Byte.MAX_VALUE),
+".* out of range", true);
+f.checkFails(String.format(Locale.US, "SELECT cast(%d+30 as smallint)",
Short.MAX_VALUE),
+".* out of range", true);
+// We use a long value because otherwise calcite interprets the literal as
an integer.
+f.checkFails(String.format(Locale.US, "SELECT cast(%d as int)",
Long.MAX_VALUE),
+".* out of range", true);
+
+// Casting a floating point value larger than the maximum allowed value.
+// 1e60 is larger than the largest BIGINT value allowed.
+f.checkFails("SELECT cast(1e60+30 as tinyint)",
+".* out of range", true);
Review Comment:
Yes, Sonar has false positives sometimes, it's fine to address only what's
reasonable (I consider fixing the complexity of an existing test method as as
nice-to-have and totally optional), but most of the time it provides good hints
for keeping the code quality high.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6111] Explicit cast from expression to numeric type doesn't check overflow [calcite]
asolimando commented on code in PR #3522:
URL: https://github.com/apache/calcite/pull/3522#discussion_r1457090549
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -643,11 +647,12 @@ void testCastExactNumericLimits(CastType castType,
SqlOperatorFixture f) {
f.checkCastToScalarOkay("'" + numeric.minNumericString + "'",
type, numeric.minNumericString, castType);
- if (Bug.CALCITE_2539_FIXED) {
+ if (numeric != Numeric.DECIMAL5_2) {
+// The above condition is for bug CALCITE-6078
f.checkCastFails("'" + numeric.maxOverflowNumericString + "'",
-type, OUT_OF_RANGE_MESSAGE, true, castType);
+type, WRONG_FORMAT_MESSAGE, true, castType);
Review Comment:
From the variable name, it seems that `OUT_OF_RANGE_MESSAGE` was the correct
error we expected (similarly to the checks in the `if` statement starting at
line 635)?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CALCITE-6208 update JSON_VALUE return type inference to make explicit array return types be nullable with nullable elements [calcite]
clintropolis commented on code in PR #3633:
URL: https://github.com/apache/calcite/pull/3633#discussion_r1457072552
##
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##
@@ -11413,6 +11413,14 @@ private void checkCustomColumnResolving(String table) {
expr("json_value('{\"foo\":100}', 'lax $.foo' returning boolean"
+ " default 100 on empty)")
.columnType("BOOLEAN");
+
+expr("json_value('{\"foo\":[100, null, 200]}', 'lax $.foo'"
++ "returning integer array)")
+.columnType("INTEGER ARRAY");
+
+expr("json_value('{\"foo\":[[100, null, 200]]}', 'lax $.foo'"
Review Comment:
I am unsure exactly of the standard; I can only speak for the behavior in
Apache Druid where we have a dedicated json type rather than operating on
strings and we coerce array elements to a common 'least restrictive type'.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CALCITE-6208 update JSON_VALUE return type inference to make explicit array return types be nullable with nullable elements [calcite]
clintropolis commented on code in PR #3633:
URL: https://github.com/apache/calcite/pull/3633#discussion_r1457062517
##
core/src/main/java/org/apache/calcite/sql/fun/SqlJsonValueFunction.java:
##
@@ -54,20 +53,33 @@ public class SqlJsonValueFunction extends SqlFunction {
public SqlJsonValueFunction(String name) {
super(name, SqlKind.OTHER_FUNCTION,
-ReturnTypes.cascade(
-opBinding ->
explicitTypeSpec(opBinding).orElse(getDefaultType(opBinding)),
-SqlTypeTransforms.FORCE_NULLABLE),
+opBinding -> explicitTypeSpec(opBinding)
+.map(relDataType -> deriveExplicitType(opBinding, relDataType))
+.orElseGet(() -> getDefaultType(opBinding)),
null,
OperandTypes.family(
ImmutableList.of(SqlTypeFamily.ANY, SqlTypeFamily.CHARACTER),
ordinal -> ordinal > 1),
SqlFunctionCategory.SYSTEM);
}
+ private static RelDataType deriveExplicitType(SqlOperatorBinding opBinding,
RelDataType type) {
+if (SqlTypeName.ARRAY == type.getSqlTypeName()) {
+ RelDataType elementType =
Objects.requireNonNull(type.getComponentType());
+ RelDataType nullableElementType = deriveExplicitType(opBinding,
elementType);
+ return SqlTypeUtil.createArrayType(
+ opBinding.getTypeFactory(),
+ nullableElementType,
+ true);
+}
+return opBinding.getTypeFactory().createTypeWithNullability(type, true);
+ }
+
/** Returns VARCHAR(2000) as default. */
private static RelDataType getDefaultType(SqlOperatorBinding opBinding) {
final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
-return typeFactory.createSqlType(SqlTypeName.VARCHAR, 2000);
+final RelDataType baseType =
typeFactory.createSqlType(SqlTypeName.VARCHAR, 2000);
Review Comment:
i don't have the context for why this was originally chosen, i was just
preserving existing behavior
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
