Re: [PR] [CALCITE-6313] Add POWER function for PostgreSQL [calcite]
mihaibudiu commented on code in PR #3762:
URL: https://github.com/apache/calcite/pull/3762#discussion_r1568072521
##
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##
@@ -2216,7 +2216,39 @@ private static RelDataType
deriveTypeMapFromEntries(SqlOperatorBinding opBinding
@LibraryOperator(libraries = {BIG_QUERY, SPARK})
public static final SqlFunction POW =
- SqlStdOperatorTable.POWER.withName("POW");
+ SqlBasicFunction.create("POW",
+ ReturnTypes.DOUBLE_NULLABLE,
+ OperandTypes.NUMERIC_NUMERIC,
+ SqlFunctionCategory.NUMERIC);
+
+ /** The {@code POWER(numeric, numeric)} function.
+ *
+ * The return type is always {@code DOUBLE} since we don't know
+ * what the result type will be by just looking at the operand types. For
+ * example {@code POWER(INTEGER, INTEGER)} can return a non-INTEGER if the
+ * second operand is negative.
+ */
+ @LibraryOperator(libraries = { BIG_QUERY, CALCITE, HIVE, MSSQL, MYSQL,
ORACLE, SNOWFLAKE,
+ SPARK })
+ public static final SqlFunction POWER =
+ SqlBasicFunction.create("POWER",
+ ReturnTypes.DOUBLE_NULLABLE,
+ OperandTypes.NUMERIC_NUMERIC,
+ SqlFunctionCategory.NUMERIC);
+
+ /** The {@code POWER(numeric, numeric)} function.
+ *
+ * The return type is always {@code DOUBLE} since we don't know
Review Comment:
this comment seems wrong
##
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##
@@ -559,15 +558,6 @@ static SqlOperatorTable operatorTableFor(SqlLibrary
library) {
}
@Test void testArithmeticOperatorsFails() {
-expr("^power(2,'abc')^")
Review Comment:
why delete these tests? What happens to them? They look like good tests.
##
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##
@@ -2006,7 +2006,7 @@ void checkPeriodPredicate(Checker checker) {
expr("1-2+3*4/5/6-7")
.ok("(((1 - 2) + (((3 * 4) / 5) / 6)) - 7)");
expr("power(2,3)")
-.ok("POWER(2, 3)");
+.ok("`POWER`(2, 3)");
Review Comment:
these quotes look suspicious
##
core/src/test/resources/sql/misc.iq:
##
@@ -2301,17 +2301,6 @@ where false;
!ok
-# [CALCITE-2447] POWER, ATAN2 functions fail with NoSuchMethodException
Review Comment:
why delete this test?
##
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##
@@ -773,6 +791,28 @@ public static SqlCall stripSeparator(SqlCall call) {
return null;
};
+ public static final SqlReturnTypeInference DECIMAL_OR_DOUBLE = opBinding -> {
Review Comment:
why have both DOUBLE_OR_DECIMAL and DECIMAL_OR_DOUBLE?
##
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##
@@ -410,6 +410,24 @@ public static SqlCall stripSeparator(SqlCall call) {
public static final SqlReturnTypeInference DOUBLE_FORCE_NULLABLE =
DOUBLE.andThen(SqlTypeTransforms.FORCE_NULLABLE);
+ /**
Review Comment:
this comment seems wrong too
--
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-6313] Add POWER function for PostgreSQL [calcite]
normanj-bitquill commented on PR #3762: URL: https://github.com/apache/calcite/pull/3762#issuecomment-2059682779 In order to separate the POWER function for PostgreSQL and non-PostgreSQL dialects, the POWER function was moved to the `SqlLibraryOperators` class. This downgrades the POWER function from a standard function to a function from a library. This is problematic for things like SQRT which is a standard function and gets rewritten to a POWER expression. Is there a more clean way to separate the POWER function so that the PostgreSQL implementation is able to return Decimal values? -- 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-6313] Add POWER function for PostgreSQL [calcite]
normanj-bitquill opened a new pull request, #3762: URL: https://github.com/apache/calcite/pull/3762 * The existing power function is moved to all non PostgreSQL libraries * The new power function is only for PostgreSQL * The new function returns a decimal if any argument is a decimal -- 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-6345] Intervals with more than 100 years are not supported [calcite]
mihaibudiu commented on code in PR #3743:
URL: https://github.com/apache/calcite/pull/3743#discussion_r1567724269
##
testkit/src/main/java/org/apache/calcite/test/IntervalTest.java:
##
@@ -870,8 +870,6 @@ public void subTestIntervalYearNegative() {
// Field value out of range
// (default, explicit default, alt, neg alt, max, neg max)
-f.wholeExpr("INTERVAL '100' YEAR")
Review Comment:
I have added the removed tests back as positive tests, but I have kept them
in the same place in the code.
I could move them to separate functions too if that's clearer.
--
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-6265] Type coercion is failing for numeric values in prepared statements Address some issues on the original patch
This is an automated email from the ASF dual-hosted git repository.
mbudiu 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 152801428f [CALCITE-6265] Type coercion is failing for numeric values
in prepared statements Address some issues on the original patch
152801428f is described below
commit 152801428fc28948d8f78753c258744f7c8e253a
Author: Ruben Quesada Lopez
AuthorDate: Fri Apr 12 10:45:18 2024 +0100
[CALCITE-6265] Type coercion is failing for numeric values in prepared
statements
Address some issues on the original patch
---
.../calcite/adapter/enumerable/EnumUtils.java | 3 +++
.../adapter/enumerable/RexToLixTranslator.java | 22 --
.../java/org/apache/calcite/test/JdbcTest.java | 27 ++
.../java/org/apache/calcite/linq4j/tree/Types.java | 25
4 files changed, 44 insertions(+), 33 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumUtils.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumUtils.java
index d4cfb7d349..7382bc 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumUtils.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumUtils.java
@@ -354,6 +354,9 @@ public class EnumUtils {
if (!Types.needTypeCast(fromType, toType)) {
return operand;
}
+
+// TODO use Expressions#convertChecked to throw exception in case of
overflow (CALCITE-6366)
+
// E.g. from "Short" to "int".
// Generate "x.intValue()".
final Primitive toPrimitive = Primitive.of(toType);
diff --git
a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
index 74a69fe924..e49788c594 100644
---
a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
+++
b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
@@ -31,7 +31,6 @@ import org.apache.calcite.linq4j.tree.Expressions;
import org.apache.calcite.linq4j.tree.ParameterExpression;
import org.apache.calcite.linq4j.tree.Primitive;
import org.apache.calcite.linq4j.tree.Statement;
-import org.apache.calcite.linq4j.tree.Types;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rex.RexBuilder;
import org.apache.calcite.rex.RexCall;
@@ -1453,20 +1452,17 @@ public class RexToLixTranslator implements
RexVisitor
// For numeric types, use java.lang.Number to prevent cast exception
// when the parameter type differs from the target type
-Expression argumentExpression =
-EnumUtils.convert(
+final Expression valueExpression = isNumeric
+? EnumUtils.convert(
+EnumUtils.convert(
+Expressions.call(root, BuiltInMethod.DATA_CONTEXT_GET.method,
+Expressions.constant("?" + dynamicParam.getIndex())),
+java.lang.Number.class),
+storageType)
+: EnumUtils.convert(
Expressions.call(root, BuiltInMethod.DATA_CONTEXT_GET.method,
Expressions.constant("?" + dynamicParam.getIndex())),
-isNumeric ? java.lang.Number.class : storageType);
-
-// Short-circuit if the expression evaluates to null. The cast
-// may throw a NullPointerException as it calls methods on the
-// object such as longValue().
-Expression valueExpression =
-Expressions.condition(
-Expressions.equal(argumentExpression, Expressions.constant(null)),
-Expressions.constant(null),
-Types.castIfNecessary(storageType, argumentExpression));
+storageType);
final ParameterExpression valueVariable =
Expressions.parameter(valueExpression.getType(),
diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
index 8fe3de21ac..023a57ef6b 100644
--- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
+++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -8453,6 +8453,33 @@ public class JdbcTest {
}
}
+ @Test void bindDecimalParameter() {
+final String sql =
+"with cte as (select 2500.55 as val)"
++ "select * from cte where val = ?";
+
+CalciteAssert.hr()
+.query(sql)
+.consumesPreparedStatement(p -> {
+ p.setBigDecimal(1, new BigDecimal("2500.55"));
+})
+.returnsUnordered("VAL=2500.55");
+ }
+
+ @Test void bindNullParameter() {
+final String sql =
+"with cte as (select 2500.55 as val)"
++ "select * from cte where val = ?";
+
+CalciteAssert.hr()
+.query(sql)
+.consumesPreparedStatement(p -> {
+ p.setBigDecimal(1, null);
+})
+.returnsUnordered("");
Re: [PR] [CALCITE-6265] Type coercion is failing for numeric values in prepared statements (follow-up) [calcite]
mihaibudiu merged PR #3758: URL: https://github.com/apache/calcite/pull/3758 -- 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-6269] Fix missing/broken BigQuery date-time format elements [calcite]
tanclary commented on code in PR #3761:
URL: https://github.com/apache/calcite/pull/3761#discussion_r1567660915
##
core/src/main/java/org/apache/calcite/util/format/FormatElementEnum.java:
##
@@ -217,6 +291,34 @@ public enum FormatElementEnum implements FormatElement {
sb.append(String.format(Locale.ROOT, "%d", (calendar.get(Calendar.MONTH)
/ 3) + 1));
}
},
+ AMPM("", "The time as Meridian Indicator in uppercase") {
+@Override public void format(StringBuilder sb, Date date) {
+ final Calendar calendar = Work.get().calendar;
+ calendar.setTime(date);
+ sb.append(calendar.get(Calendar.AM_PM) == Calendar.AM ? "AM" : "PM");
Review Comment:
can you just call `.toString()` on calendar.get(am_pm)` or no
##
core/src/main/java/org/apache/calcite/util/format/FormatElementEnum.java:
##
@@ -306,12 +446,22 @@ static Work get() {
* https://issues.apache.org/jira/browse/CALCITE-6252. This may be
* specific to Java 11. */
final DateFormat Format = new SimpleDateFormat(MONTH.javaFmt,
Locale.US);
-final DateFormat sFormat = new SimpleDateFormat(FF1.javaFmt, Locale.ROOT);
-final DateFormat ssFormat = new SimpleDateFormat(FF2.javaFmt, Locale.ROOT);
final DateFormat sssFormat = new SimpleDateFormat(FF3.javaFmt,
Locale.ROOT);
-final DateFormat Format = new SimpleDateFormat(FF4.javaFmt,
Locale.ROOT);
-final DateFormat sFormat = new SimpleDateFormat(FF5.javaFmt,
Locale.ROOT);
-final DateFormat ssFormat = new SimpleDateFormat(FF6.javaFmt,
Locale.ROOT);
final DateFormat yyFormat = new SimpleDateFormat(YY.javaFmt, Locale.ROOT);
+final DateFormat Format = new SimpleDateFormat(.javaFmt,
Locale.ROOT);
+
+/** Util to return the full or abbreviated weekday name from date and
expected TextStyle. */
+public String getDayFromDate(Date date, TextStyle style) {
Review Comment:
should this be private
--
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-6265] Type coercion is failing for numeric values in prepared statements (follow-up) [calcite]
sonarcloud[bot] commented on PR #3758: URL: https://github.com/apache/calcite/pull/3758#issuecomment-2058480696 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3758) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3758=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite=3758=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3758=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3758=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3758=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3758) -- 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-6265] Type coercion is failing for numeric values in prepared statements (follow-up) [calcite]
rubenada commented on code in PR #3758:
URL: https://github.com/apache/calcite/pull/3758#discussion_r1566855343
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##
@@ -1393,20 +1392,17 @@ private Result toInnerStorageType(Result result, Type
storageType) {
// For numeric types, use java.lang.Number to prevent cast exception
// when the parameter type differs from the target type
-Expression argumentExpression =
-EnumUtils.convert(
+final Expression valueExpression = isNumeric
+? EnumUtils.convert(
+EnumUtils.convert(
+Expressions.call(root, BuiltInMethod.DATA_CONTEXT_GET.method,
+Expressions.constant("?" + dynamicParam.getIndex())),
+java.lang.Number.class),
+storageType)
+: EnumUtils.convert(
Expressions.call(root, BuiltInMethod.DATA_CONTEXT_GET.method,
Expressions.constant("?" + dynamicParam.getIndex())),
-isNumeric ? java.lang.Number.class : storageType);
-
-// Short-circuit if the expression evaluates to null. The cast
-// may throw a NullPointerException as it calls methods on the
-// object such as longValue().
-Expression valueExpression =
Review Comment:
Good point. I have added a new test for `null`. It works fine, it seems the
existing logic on `EnumUtils#convert` already handled that scenario correctly,
but it's good to have an explicit test confirming it.
--
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-6265] Type coercion is failing for numeric values in prepared statements (follow-up) [calcite]
rubenada commented on code in PR #3758:
URL: https://github.com/apache/calcite/pull/3758#discussion_r1566855343
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##
@@ -1393,20 +1392,17 @@ private Result toInnerStorageType(Result result, Type
storageType) {
// For numeric types, use java.lang.Number to prevent cast exception
// when the parameter type differs from the target type
-Expression argumentExpression =
-EnumUtils.convert(
+final Expression valueExpression = isNumeric
+? EnumUtils.convert(
+EnumUtils.convert(
+Expressions.call(root, BuiltInMethod.DATA_CONTEXT_GET.method,
+Expressions.constant("?" + dynamicParam.getIndex())),
+java.lang.Number.class),
+storageType)
+: EnumUtils.convert(
Expressions.call(root, BuiltInMethod.DATA_CONTEXT_GET.method,
Expressions.constant("?" + dynamicParam.getIndex())),
-isNumeric ? java.lang.Number.class : storageType);
-
-// Short-circuit if the expression evaluates to null. The cast
-// may throw a NullPointerException as it calls methods on the
-// object such as longValue().
-Expression valueExpression =
Review Comment:
Good point. I have added a new test for `null`. It works fine, it seems the
existing logic on EnumUtils#convert already handled that scenario correctly,
but it's good to have a explicit test confirming it.
--
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
