[PR] Making code example more compilable [calcite-site]
kenmy opened a new pull request, #6: URL: https://github.com/apache/calcite-site/pull/6 Line of code: `public final Employee[] emps = 0;` is not valid from java side. It was changed to `public final Employee[] emps = new Employee[0];` This line can be compiled by Java compiler -- 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-6219] Must Filter validation [calcite]
sonarcloud[bot] commented on PR #3688: URL: https://github.com/apache/calcite/pull/3688#issuecomment-1945105906 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3688) **Quality Gate passed** Issues  [15 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3688=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3688=false=true)  [82.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3688=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3688=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3688) -- 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-6210] Cast to VARBINARY causes an assertion failure [calcite]
julianhyde commented on PR #3635: URL: https://github.com/apache/calcite/pull/3635#issuecomment-1944951368 @gortiz If necessary, you could create a new jira account. We lean rather heavily on Jira in Calcite because Jira provides a permanent record of design discussion that can be searched and cross-referenced. -- 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] Allow providing different numeric type as placeholder [calcite]
asolimando commented on code in PR #3687:
URL: https://github.com/apache/calcite/pull/3687#discussion_r1490147805
##
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##
@@ -8301,6 +8301,67 @@ private void checkGetTimestamp(Connection con) throws
SQLException {
});
}
+ final String[] intTypes = {"tinyint", "smallint", "int", "bigint"};
Review Comment:
You can use `SqlTypeFamily.INTEGER` to derive those type names
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##
@@ -1374,11 +1374,35 @@ private Result toInnerStorageType(Result result, Type
storageType) {
}
final Type storageType = currentStorageType != null
? currentStorageType :
typeFactory.getJavaClass(dynamicParam.getType());
-final Expression valueExpression =
+
+// For numeric types, get the value using the following functions on the
Review Comment:
As @mihaibudiu suggests, this is basically a cast, which is already
available via the `Types.castIfNecessary` method, see below for how it can be
used:
```
final boolean isNumeric =
SqlTypeFamily.NUMERIC.contains(dynamicParam.getType());
final Expression valueExpression = Types.castIfNecessary(storageType,
EnumUtils.convert(Expressions.call(root,
BuiltInMethod.DATA_CONTEXT_GET.method,
Expressions.constant("?" + dynamicParam.getIndex())),
isNumeric ? java.lang.Number.class : storageType));
```
If you replace your code with this, it passes all the tests you added, and
it might be working for non-numeric cases too (maybe with some adaptations).
Could you give it a try and add more tests non-numeric types to check?
--
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] Allow providing different numeric type as placeholder [calcite]
tindzk commented on code in PR #3687:
URL: https://github.com/apache/calcite/pull/3687#discussion_r1489933074
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##
@@ -1374,11 +1374,35 @@ private Result toInnerStorageType(Result result, Type
storageType) {
}
final Type storageType = currentStorageType != null
? currentStorageType :
typeFactory.getJavaClass(dynamicParam.getType());
-final Expression valueExpression =
+
+// For numeric types, get the value using the following functions on the
Review Comment:
Indeed. The PR only fixes the issue for numeric values, but for other types
the user would still encounter a `ClassCastException`. Ideally, we would have a
function that handles all implicit conversions and fails with a user-friendly
error for unsupported conversions. I am sure this functionality already exists
in the Calcite in the form of `CAST` expressions. Possibly it could be
leveraged here.
--
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] Allow providing different numeric type as placeholder [calcite]
mihaibudiu commented on code in PR #3687:
URL: https://github.com/apache/calcite/pull/3687#discussion_r1489891786
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##
@@ -1374,11 +1374,35 @@ private Result toInnerStorageType(Result result, Type
storageType) {
}
final Type storageType = currentStorageType != null
? currentStorageType :
typeFactory.getJavaClass(dynamicParam.getType());
-final Expression valueExpression =
+
+// For numeric types, get the value using the following functions on the
Review Comment:
This may work for numeric values, but does not seem to be a general solution
for other types. Is there a design that would handle that? It looks like any
source/destination type combination is possible. So perhaps this could be
handled like a cast expression? Although I don't see the destination type in
this function.
--
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-6157] SqlValidatorImpl breaks join condition when inlining table alias [calcite]
mihaibudiu commented on code in PR #3686:
URL: https://github.com/apache/calcite/pull/3686#discussion_r1489867427
##
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##
@@ -2276,6 +2276,12 @@ protected void registerNamespace(
if (namespace == null) {
namespaces.put(requireNonNull(ns.getNode()), ns);
namespace = ns;
+} else {
Review Comment:
This code does not respect the style guidelines of Calcite.
`./gradlew build` will show you the problems.
However, more importantly, this does not look like a suitable error message
for a user of Calcite.
Is this an internal error or the compiler? Then perhaps you should use an
assertion.
Even better would be to fix the error. The issue you have filed suggests
that you are fixing the problem, but it seems that you aren't.
--
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 6244] Allow `Expressions#constant` to construct Immutable class models [calcite]
sonarcloud[bot] commented on PR #3681: URL: https://github.com/apache/calcite/pull/3681#issuecomment-1944301017 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3681) **Quality Gate passed** Issues  [3 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3681=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3681=false=true)  [72.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3681=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3681=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3681) -- 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 6244] Allow `Expressions#constant` to construct Immutable class models [calcite]
sonarcloud[bot] commented on PR #3681: URL: https://github.com/apache/calcite/pull/3681#issuecomment-1944275921 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3681) **Quality Gate passed** Issues  [3 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3681=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3681=false=true)  [72.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3681=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3681=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3681) -- 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] Allow providing different numeric type as placeholder [calcite]
sonarcloud[bot] commented on PR #3687: URL: https://github.com/apache/calcite/pull/3687#issuecomment-1944266017 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3687) **Quality Gate passed** Issues  [4 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3687=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3687=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3687=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3687=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3687) -- 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 6244] Allow `Expressions#constant` to construct Immutable class models [calcite]
sonarcloud[bot] commented on PR #3681: URL: https://github.com/apache/calcite/pull/3681#issuecomment-1944263637 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3681) **Quality Gate passed** Issues  [3 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3681=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3681=false=true)  [72.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3681=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3681=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3681) -- 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-6265] Allow providing different numeric type as placeholder [calcite]
tindzk opened a new pull request, #3687: URL: https://github.com/apache/calcite/pull/3687 (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 6244] Allow `Expressions#constant` to construct Immutable class models [calcite]
sonarcloud[bot] commented on PR #3681: URL: https://github.com/apache/calcite/pull/3681#issuecomment-1944201234 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3681) **Quality Gate passed** Issues  [3 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3681=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3681=false=true)  [72.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3681=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3681=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3681) -- 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-6249] RelNode::estimatedRowCount should not be used in computeSelfCost [calcite]
tanclary commented on PR #3674: URL: https://github.com/apache/calcite/pull/3674#issuecomment-1944194775 > > The title says something should not be used, but there is no test to tell me that the claim of the JIRA case is true or has been fixed by this PR. > > In the current code base, `mq.getRowCount` is used in most cases inside `computeSelfCost`. On the other hand, `estimatedRowCount` is only used in some rare cases, which are fixed by this PR. As @rubenada already pointed out, there is also a javadoc, which states that the claim of the JIRA case is true. In my point of view, there is no need to prove this by a unit test. A javadoc and a JIRA case don't have any impact on the execution of certain code though. I'm not trying to block this commit from getting merged, I just don't think it's good practice to make changes without adding tests, especially when there's no existing tests. -- 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-6239] Add a postgis dialect that supports ST functions [calcite]
sonarcloud[bot] commented on PR #3668: URL: https://github.com/apache/calcite/pull/3668#issuecomment-1943793419 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3668) **Quality Gate passed** Issues  [14 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3668=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3668=false=true)  [84.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3668=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3668=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3668) -- 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-6239] Add a postgis dialect that supports ST functions [calcite]
sonarcloud[bot] commented on PR #3668: URL: https://github.com/apache/calcite/pull/3668#issuecomment-1943793834 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3668) **Quality Gate passed** Issues  [12 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3668=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3668=false=true)  [84.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3668=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3668=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3668) -- 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-6239] Add a postgis dialect that supports ST functions [calcite]
sonarcloud[bot] commented on PR #3668: URL: https://github.com/apache/calcite/pull/3668#issuecomment-1943689440 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3668) **Quality Gate passed** Issues  [12 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3668=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3668=false=true)  [84.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3668=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3668=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3668) -- 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-6239] Add a postgis dialect that supports ST functions [calcite]
sonarcloud[bot] commented on PR #3668: URL: https://github.com/apache/calcite/pull/3668#issuecomment-1943465882 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3668) **Quality Gate passed** Issues  [12 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3668=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3668=false=true)  [84.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3668=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3668=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3668) -- 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-6239] Add a postgis dialect that supports ST functions [calcite]
sonarcloud[bot] commented on PR #3668: URL: https://github.com/apache/calcite/pull/3668#issuecomment-1943428367 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3668) **Quality Gate passed** Issues  [12 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3668=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3668=false=true)  [84.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3668=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3668=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3668) -- 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-6239] Add a postgis dialect that supports ST functions [calcite]
bchapuis commented on code in PR #3668: URL: https://github.com/apache/calcite/pull/3668#discussion_r1489118497 ## core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactoryImpl.java: ## @@ -39,9 +39,11 @@ import com.google.common.collect.Interners; import org.checkerframework.checker.nullness.qual.Nullable; +import org.locationtech.jts.geom.Geometry; Review Comment: Yes, here is the jira issue I just created: https://issues.apache.org/jira/browse/CALCITE-6263 -- 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-6157] SqlValidatorImpl breaks join condition when inlining table alias [calcite]
TheOezzi opened a new pull request, #3686: URL: https://github.com/apache/calcite/pull/3686 (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
