Re: [PR] [CALCITE-6223] Add MAP_CONTAINS_KEY function (enabled in Spark library) [calcite]
caicancai commented on PR #3655: URL: https://github.com/apache/calcite/pull/3655#issuecomment-1926402731 > I think as a committer of an Apache project, you must be an excellent engineer, but in this PR, you have made too many low-level errors, and even copied a lot of code from other places. I hope you can double check your code before submitting the PR. > > I'm sorry for being a bit harsh, but I hope to save time for all contributors (including reviewers) in the Calcite community, as most of us participate in community activities during our personal time. > > Another thing, try not to use rebase or square unless requested by the committer, otherwise reviewers can not check the diff between commits. > > > In order to update the pull request, you need to commit the changes in your branch and then push the commit(s) to GitHub. You are encouraged to use regular (non-rebased) commits on top of previously existing ones. > > When pushing the changes to GitHub, you should refrain from using the --force parameter and its alternatives. You may choose to force push your changes under certain conditions: > > the pull request has been submitted less than 10 minutes ago and there is no pending discussion (in the PR and/or in JIRA) concerning it; > > a reviewer has explicitly asked you to perform some modifications that require the use of the --force option. > > See details in [develop guide](https://calcite.apache.org/develop/). Thank you very much for your suggestions. Recently, the PRs of my two adaptation functions have caused a lot of trouble for everyone. In other words, some of my recent PRs have more or less made some low-level mistakes. I am also reflecting on myself and causing you problems. I'm sorry for bothering you, I have to admit I've been a little anxious lately. I won't make similar low-level mistakes again in the future. -- 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-6223] Add MAP_CONTAINS_KEY function (enabled in Spark library) [calcite]
macroguo-ghy commented on code in PR #3655:
URL: https://github.com/apache/calcite/pull/3655#discussion_r1477751412
##
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##
@@ -1468,6 +1468,13 @@ private static RelDataType
deriveTypeMapConcat(SqlOperatorBinding opBinding) {
ReturnTypes.TO_MAP_VALUES_NULLABLE,
OperandTypes.MAP);
+ /** The "MAP_CONTAINS_KEY(map, key)" function. */
+ @LibraryOperator(libraries = {SPARK})
+ public static final SqlFunction MAP_CONTAINS_KEYS =
Review Comment:
`MAP_CONTAINS_KEYS` -> `MAP_CONTAINS_KEY`?
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -7216,6 +7216,43 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
"INTEGER ARRAY NOT NULL");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6223;>[CALCITE-6223]
+ * Add MAP_CONTAINS_KEY function (enabled in BigQuery library).
Review Comment:
BigQuery?
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -844,6 +845,7 @@ Builder populate2() {
defineMethod(ARRAYS_ZIP, BuiltInMethod.ARRAYS_ZIP.method,
NullPolicy.ANY);
defineMethod(EXISTS, BuiltInMethod.EXISTS.method, NullPolicy.ANY);
defineMethod(MAP_CONCAT, BuiltInMethod.MAP_CONCAT.method,
NullPolicy.ANY);
+ defineMethod(MAP_CONTAINS_KEYS, BuiltInMethod.MAP_CONTAINS_KEYS.method,
NullPolicy.ANY);
Review Comment:
`MAP_CONTAINS_KEYS` -> `MAP_CONTAINS_KEY`?
--
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] Upgrade Cassandra to 4.1.3 and DataStax driver for Cassandra to 4.17.0 [calcite]
asolimando commented on code in PR #3670:
URL: https://github.com/apache/calcite/pull/3670#discussion_r1477725247
##
cassandra/src/test/java/org/apache/calcite/test/CassandraExtension.java:
##
@@ -175,14 +189,6 @@ private CassandraResource() {
Thread.currentThread().interrupt();
}
Stage.shutdownNow();
-
- if (FBUtilities.isWindows) {
Review Comment:
That's correct, those were Windows-specific methods that C* dropped when
they stopped supporting 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-6223] Add MAP_CONTAINS_KEY function (enabled in Spark library) [calcite]
caicancai commented on PR #3655: URL: https://github.com/apache/calcite/pull/3655#issuecomment-1926304579 @macroguo-ghy Would you mind helping me take a look, thank you -- 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-2040] Create adapter for Apache Arrow [calcite]
sonarcloud[bot] commented on PR #3666: URL: https://github.com/apache/calcite/pull/3666#issuecomment-1926292316 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3666) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [21 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3666=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3666=false=true) [84.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3666=new_coverage=list) [3.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3666=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3666) -- 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-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477705614
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6215,6 +6215,71 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+final SqlOperatorFixture f0 = fixture();
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",
Review Comment:
Totally agree, if necessary we can do something similar to the `SUBSTR`
functions which I think is implemented per-dialect.
--
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] Add .gitignore and .ratignore for jenv [calcite]
sonarcloud[bot] commented on PR #3671: URL: https://github.com/apache/calcite/pull/3671#issuecomment-1926211890 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3671) **Quality Gate passed** Kudos, no new issues were introduced! [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3671=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3671=false=true) No data about Coverage No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3671) -- 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] Add .gitignore and .ratignore for jenv [calcite]
macroguo-ghy opened a new pull request, #3671: URL: https://github.com/apache/calcite/pull/3671 When use jenv manage the local java environment, it will generate a `.java-version` file, we should ignore it. See more details about jenv in https://github.com/jenv/jenv. -- 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-6241] Add a few existing functions to Spark library [calcite]
YiwenWu commented on PR #3669: URL: https://github.com/apache/calcite/pull/3669#issuecomment-1926099902 > > > > Just a question, how to ensure that the semantics of these functions are consistent with those in SPARK? > > > > > > > > > @macroguo-ghy I executed the Function Sqls in the spark engine. And checked the function Definition/OperandType/ReturnType is consistent with the Spark definition. > > > > > > Some functions have subtle differences for corner cases of the arguments. And Calcite tests often do _not_ test the corner cases. I haven't checked Spark in particular, but we are writing a lot of tests for existing functions and we find lots of such cases. > > In fact, it's difficult for us to handle all situations. There are also slight differences in functions between different Spark versions. Many behaviors should be left to users to decide based on their Spark version. yes，Engine version is another question that also confuses us -- 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-6241] Add a few existing functions to Spark library [calcite]
YiwenWu commented on PR #3669: URL: https://github.com/apache/calcite/pull/3669#issuecomment-1926097975 > > > Just a question, how to ensure that the semantics of these functions are consistent with those in SPARK? > > > > > > @macroguo-ghy I executed the Function Sqls in the spark engine. And checked the function Definition/OperandType/ReturnType is consistent with the Spark definition. > > Some functions have subtle differences for corner cases of the arguments. And Calcite tests often do _not_ test the corner cases. I haven't checked Spark in particular, but we are writing a lot of tests for existing functions and we find lots of such cases. Strongly agree, there may be many subtle differences in the execution of functions of each engine. This problem also troubles us very much, For example, precision issues and null value issues。 In my opinion, the current function definition cannot guarantee the correctness of execution, mainly limiting the input types and return types of 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-6241] Add a few existing functions to Spark library [calcite]
mihaibudiu commented on PR #3669: URL: https://github.com/apache/calcite/pull/3669#issuecomment-1926090656 If you use some of the REDUCE optimizations, they will simplify constant expressions involving these functions. -- 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-6241] Add a few existing functions to Spark library [calcite]
JiajunBernoulli commented on PR #3669: URL: https://github.com/apache/calcite/pull/3669#issuecomment-1926087960 > > > Just a question, how to ensure that the semantics of these functions are consistent with those in SPARK? > > > > > > @macroguo-ghy I executed the Function Sqls in the spark engine. And checked the function Definition/OperandType/ReturnType is consistent with the Spark definition. > > Some functions have subtle differences for corner cases of the arguments. And Calcite tests often do _not_ test the corner cases. I haven't checked Spark in particular, but we are writing a lot of tests for existing functions and we find lots of such cases. In fact, it's difficult for us to handle all situations. There are also slight differences in functions between different Spark versions. Many behaviors should be left to users to decide based on their Spark version. -- 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]
JiajunBernoulli commented on PR #3668: URL: https://github.com/apache/calcite/pull/3668#issuecomment-1926081457 > @YiwenWu @JiajunBernoulli What would be the best way to test these changes against a postgis database? Ideally, I'd like to add an integration test that executes queries against postgis (e.g. with testcontainers). But I havn't been able to find such tests in calcite. There are some integration tests for Druid: https://github.com/zabetak/calcite-druid-dataset Here is CI config in calcite: https://github.com/apache/calcite/blob/2aabf210dc1918c6ca20e63b39661ff445535eb8/.github/workflows/main.yml#L440 -- 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] Upgrade Cassandra to 4.1.3 and DataStax driver for Cassandra to 4.17.0 [calcite]
JiajunBernoulli commented on code in PR #3670:
URL: https://github.com/apache/calcite/pull/3670#discussion_r1477557164
##
cassandra/src/test/java/org/apache/calcite/test/CassandraExtension.java:
##
@@ -175,14 +189,6 @@ private CassandraResource() {
Thread.currentThread().interrupt();
}
Stage.shutdownNow();
-
- if (FBUtilities.isWindows) {
Review Comment:
Is this because Cassandra dropped Windows?
--
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-site) branch main updated: Website deployed from calcite-avatica@178ff82caeb415d38c5852b8f6c7373bd8d97718
This is an automated email from the ASF dual-hosted git repository. asf-ci-deploy pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite-site.git The following commit(s) were added to refs/heads/main by this push: new 1e0c0ecc3 Website deployed from calcite-avatica@178ff82caeb415d38c5852b8f6c7373bd8d97718 1e0c0ecc3 is described below commit 1e0c0ecc389e7bc8a2d9be1e33396e6391c8c8f3 Author: F21 AuthorDate: Mon Feb 5 01:21:45 2024 + Website deployed from calcite-avatica@178ff82caeb415d38c5852b8f6c7373bd8d97718 --- avatica/develop/avatica-go.html | 2 +- avatica/develop/avatica.html| 2 +- avatica/develop/index.html | 2 +- avatica/docs/client_reference.html | 2 +- avatica/docs/compatibility.html | 2 +- avatica/docs/custom_client_artifacts.html | 2 +- avatica/docs/docker.html| 2 +- avatica/docs/go_client_reference.html | 2 +- avatica/docs/go_history.html| 2 +- avatica/docs/go_howto.html | 2 +- avatica/docs/history.html | 2 +- avatica/docs/howto.html | 2 +- avatica/docs/index.html | 11 ++- avatica/docs/json_reference.html| 2 +- avatica/docs/protobuf_reference.html| 2 +- avatica/docs/protocol_testing.html | 2 +- avatica/docs/roadmap.html | 2 +- avatica/docs/security.html | 2 +- avatica/downloads/avatica-go.html | 2 +- avatica/downloads/avatica.html | 2 +- avatica/downloads/index.html| 2 +- avatica/index.html | 2 +- avatica/news/2016/03/04/separate-project/index.html | 2 +- avatica/news/2016/03/18/release-1.7.1/index.html| 2 +- avatica/news/2016/06/04/release-1.8.0/index.html| 2 +- avatica/news/2016/11/01/release-1.9.0/index.html| 2 +- avatica/news/2017/03/31/new-avatica-repository/index.html | 2 +- avatica/news/2017/05/30/release-1.10.0/index.html | 2 +- avatica/news/2018/03/09/release-1.11.0/index.html | 2 +- avatica/news/2018/04/27/release-avatica-go-3.0.0/index.html | 2 +- avatica/news/2018/06/24/release-1.12.0/index.html | 2 +- avatica/news/2018/09/10/release-avatica-go-3.1.0/index.html | 2 +- avatica/news/2018/09/18/release-avatica-go-3.2.0/index.html | 2 +- avatica/news/2018/12/04/release-1.13.0/index.html | 2 +- avatica/news/2019/04/29/release-1.14.0/index.html | 2 +- avatica/news/2019/05/13/release-1.15.0/index.html | 2 +- avatica/news/2019/05/16/release-avatica-go-4.0.0/index.html | 2 +- avatica/news/2019/12/19/release-1.16.0/index.html | 2 +- avatica/news/2020/06/22/release-1.17.0/index.html | 2 +- avatica/news/2020/07/16/release-avatica-go-5.0.0/index.html | 2 +- avatica/news/2021/05/18/release-1.18.0/index.html | 2 +- avatica/news/2021/10/11/release-1.19.0/index.html | 2 +- avatica/news/2021/12/13/release-1.20.0/index.html | 2 +- avatica/news/2022/03/27/release-avatica-go-5.1.0/index.html | 2 +- avatica/news/2022/05/08/release-1.21.0/index.html | 2 +- avatica/news/2022/07/28/release-1.22.0/index.html | 2 +- avatica/news/2022/10/13/release-avatica-go-5.2.0/index.html | 2 +- avatica/news/2023/01/19/release-1.23.0/index.html | 2 +- avatica/news/2023/12/04/release-1.24.0/index.html | 2 +- avatica/news/2023/12/11/release-avatica-go-5.3.0/index.html | 2 +- avatica/news/avatica-go-releases/index.html | 2 +- avatica/news/avatica-releases/index.html| 2 +- avatica/news/index.html | 2 +- 53 files changed, 62 insertions(+), 53 deletions(-) diff --git a/avatica/develop/avatica-go.html b/avatica/develop/avatica-go.html index 0a0682b1d..20d441d75 100644 --- a/avatica/develop/avatica-go.html +++ b/avatica/develop/avatica-go.html @@ -182,7 +182,7 @@ PHOENIX_HOST: http://phoenix:8765 - The contents of this website are 2023 + The contents of this website are 2024 https://www.apache.org/;>Apache Software Foundation under the terms of the https://www.apache.org/licenses/LICENSE-2.0.html;> diff --git a/avatica/develop/avatica.html b/avatica/develop/avatica.html index 5dc52f841..cb5a5c6a9 100644 --- a/avatica/develop/avatica.html +++ b/avatica/develop/avatica.html @@ -185,7 +185,7 @@ or just by answering
(calcite-avatica) branch main updated: Add Calcite CLI tool to list of Avatica Clients on website
This is an automated email from the ASF dual-hosted git repository. francischuang 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 178ff82ca Add Calcite CLI tool to list of Avatica Clients on website 178ff82ca is described below commit 178ff82caeb415d38c5852b8f6c7373bd8d97718 Author: Satya Kommula AuthorDate: Mon Feb 5 06:48:29 2024 +0530 Add Calcite CLI tool to list of Avatica Clients on website --- site/_docs/index.md | 7 +++ 1 file changed, 7 insertions(+) diff --git a/site/_docs/index.md b/site/_docs/index.md index 83259b4a9..02dfd346f 100644 --- a/site/_docs/index.md +++ b/site/_docs/index.md @@ -181,3 +181,10 @@ highly welcomed! * *License*: [MIT](https://opensource.org/licenses/MIT) * Any Avatica version * *Maintainer*: Waylay.io + +### Calcite Avatica CLI: A Go-based Toool +* [Home page](https://github.com/satyakommula96/calcite-cli) +* Language: Go +* *License*: [Apache 2.0](https://www.apache.org/licenses/LICENSE-2.0) +* Avatica version 1.8.0 onwards +* *Maintainer*: [Satya Kommula](https://github.com/satyakommula96)
Re: [PR] Add Calcite CLI tool to list of Avatica Clients on website [calcite-avatica]
F21 merged PR #237: URL: https://github.com/apache/calcite-avatica/pull/237 -- 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 Avatica CLI tool [calcite-avatica]
F21 commented on PR #237: URL: https://github.com/apache/calcite-avatica/pull/237#issuecomment-1926045522 Looks good. Please squash all your commits and update the commit message so that it's more descriptive. Maybe something like: `Add Calcite CLI tool to list of Avatica Clients on website` -- 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 Avatica CLI tool [calcite-avatica]
F21 commented on code in PR #237: URL: https://github.com/apache/calcite-avatica/pull/237#discussion_r1477447637 ## site/_docs/index.md: ## @@ -181,3 +181,10 @@ highly welcomed! * *License*: [MIT](https://opensource.org/licenses/MIT) * Any Avatica version * *Maintainer*: Waylay.io + +### Calcite Avatica CLI: A Go-based Toool +* [Home page](https://github.com/satyakommula96/calcite-cli) +* Language: GO Review Comment: Use `Go` rather than `GO` ## site/_docs/index.md: ## @@ -181,3 +181,10 @@ highly welcomed! * *License*: [MIT](https://opensource.org/licenses/MIT) * Any Avatica version * *Maintainer*: Waylay.io + +### Calcite Avatica CLI: A Go-based Toool +* [Home page](https://github.com/satyakommula96/calcite-cli) +* Language: GO +* *License*: [Apache 2.0](https://www.apache.org/licenses/LICENSE-2.0) +* Any Avatica version Review Comment: As the CLI uses calcite-avatica-go, I think the supported versions should be `1.8.0 onwards` ans these are the versions supported by the Go driver. -- 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-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]
mihaibudiu commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477438271
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6215,6 +6215,71 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+final SqlOperatorFixture f0 = fixture();
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",
Review Comment:
If a function is part of a dialect, it should definitely have the same
behavior as the dialect requires. This should also be documented. We may be
dealing with multiple LOG functions, with slightly different behaviors. That's
why it is very important to test all corner cases, including writing negative
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-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477424593
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6215,6 +6215,71 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+final SqlOperatorFixture f0 = fixture();
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",
Review Comment:
I am still of the belief that even though LOG(0) returning `-Infinity` makes
sense mathematically, we should do our best to parity the dialects that support
these functions (i.e. return NULL or error, depending on the dialect). I am a
fan of extending a function's behavior beyond whatever the dialect supports,
but not augmenting 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-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477424593
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6215,6 +6215,71 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+final SqlOperatorFixture f0 = fixture();
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",
Review Comment:
I am still of the belief that even though LOG(0) returning `-Infinity` makes
sense mathematically, we should do our best to parity the dialects that support
these functions (i.e. return NULL or error, depending on the dialect). I am a
fan of extending a function's behavior, but not augmenting 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-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477424593
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6215,6 +6215,71 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+final SqlOperatorFixture f0 = fixture();
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",
Review Comment:
I am still of the belief that even though LOG(0) returning `-Infinity` makes
sense mathematically, we should do our best to parity the dialects that support
these functions. I am a fan of extending a function's behavior, but not
augmenting 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-6241] Add a few existing functions to Spark library [calcite]
mihaibudiu commented on PR #3669: URL: https://github.com/apache/calcite/pull/3669#issuecomment-1925877552 > > Just a question, how to ensure that the semantics of these functions are consistent with those in SPARK? > > @macroguo-ghy I executed the Function Sqls in the spark engine. And checked the function Definition/OperandType/ReturnType is consistent with the Spark definition. Some functions have subtle differences for corner cases of the arguments. And Calcite tests often do *not* test the corner cases. I haven't checked Spark in particular, but we are writing a lot of tests for existing functions and we find lots of such cases. -- 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-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]
mihaibudiu commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477418754
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6215,6 +6215,71 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+final SqlOperatorFixture f0 = fixture();
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",
Review Comment:
I would recommend studying the SQL language, otherwise you will have
surprises.
It's very difficult to contribute to a compiler if you don't understand the
language you are compiling.
In SQL floating point is generally not used. SQL has a family of DECIMAL (or
NUMERIC -- same thing) data types, which are very different from floating
point. 2.0 is a DECIMAL literal. DECIMAL is not a type, it's a family of types
with different scales and precisions. Unfortunately, different dialects of SQL
treat DECIMAL in incompatible ways. Floating point is an IEEE standard, but
DECIMAL isn'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] Upgrade Cassandra to 4.1.3 and DataStax driver for Cassandra to 4.17.0 [calcite]
sonarcloud[bot] commented on PR #3670: URL: https://github.com/apache/calcite/pull/3670#issuecomment-1925864693 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3670) **Quality Gate passed** Kudos, no new issues were introduced! [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3670=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3670=false=true) No data about Coverage No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3670) -- 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-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]
sonarcloud[bot] commented on PR #3648: URL: https://github.com/apache/calcite/pull/3648#issuecomment-1925771662 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3648) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite=3648=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3648=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3648=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3648=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3648) -- 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-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477247017
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6215,6 +6215,71 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+final SqlOperatorFixture f0 = fixture();
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",
Review Comment:
Hello, I seem to have forgotten the simplest truth, that is, there are no
fractions in computers, only float/double point numbers. I changed 2/3 in the
test to 2.0/3, and it turns out that the result is correct. If 2/3 must output
the correct result, then this bug does not only exist in the log function, but
the pow and abs functions also have similar problems. Regarding log(0) I don't
think it's a bug.
Thank you very much for your guidance on these two PRs of my adaptation
function. I am sorry for causing you a lot of trouble. If there is anything
missing in this PR, please point it out, thank you.
--
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-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477310637
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6215,6 +6215,71 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+final SqlOperatorFixture f0 = fixture();
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",
Review Comment:
I will remove this part of the test later
--
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 Avatica CLI tool [calcite-avatica]
satyakommula96 opened a new pull request, #237: URL: https://github.com/apache/calcite-avatica/pull/237 Please review the initial integration of the Calcite CLI tool using Go in list of clients . If any adjustments are needed, kindly let me know. @F21 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-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_r1477257234
##
core/src/main/java/org/apache/calcite/sql/dialect/PostgisSqlDialect.java:
##
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.sql.dialect;
+
+import org.apache.calcite.avatica.util.Casing;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.util.RelToSqlConverterUtil;
+
+/**
+ * A SqlDialect implementation for the PostgreSQL database.
+ */
+public class PostgisSqlDialect extends PostgresqlSqlDialect {
+
+ public static final SqlDialect.Context DEFAULT_CONTEXT =
SqlDialect.EMPTY_CONTEXT
+ .withDatabaseProduct(DatabaseProduct.POSTGIS)
+ .withIdentifierQuoteString("\"")
+ .withUnquotedCasing(Casing.TO_LOWER)
+ .withDataTypeSystem(POSTGRESQL_TYPE_SYSTEM);
+
+ public static final SqlDialect DEFAULT = new
PostgisSqlDialect(DEFAULT_CONTEXT);
+
+ /** Creates a PostgresqlSqlDialect. */
+ public PostgisSqlDialect(Context context) {
+super(context);
+ }
+
+ @Override public void unparseCall(SqlWriter writer, SqlCall call, int
leftPrec, int rightPrec) {
Review Comment:
ST_UNION is the main case I had in mind that required to introduce a new
function name (ST_UNARYUNION) in calcite. However, some other functions, such
as ST_DUMP, may require a similar treatment. I guess that the list will grow as
the dialect gets more systematically tested against postgis.
--
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-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477247017
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6215,6 +6215,71 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+final SqlOperatorFixture f0 = fixture();
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",
Review Comment:
@mihaibudiu @tanclary Hello, I seem to have forgotten the simplest truth,
that is, there are no fractions in computers, only float/double point numbers.
I changed 2/3 in the test to 2.0/3, and it turns out that the result is
correct. If 2/3 must output the correct result, then this bug does not only
exist in the log function, but the pow and abs functions also have similar
problems. Regarding log(0) I don't think it's a bug.
Thank you very much for your guidance on these two PRs of my adaptation
function. I am sorry for causing you a lot of trouble. If there is anything
missing in this PR, please point it out, thank you.
--
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-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477247017
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6215,6 +6215,71 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+final SqlOperatorFixture f0 = fixture();
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",
Review Comment:
Hello, I seem to have forgotten the simplest truth, that is, there are no
fractions in computers, only float/double point numbers. I changed 2/3 in the
test to 2.0/3, and it turns out that the result is correct. If 2/3 must output
the correct result, then this bug does not only exist in the log function, but
the pow and abs functions also have similar problems. Regarding log(0) I don't
think it's a bug.
Thank you very much for your guidance on these two PRs of my adaptation
function. I am sorry for causing you a lot of trouble. If there is anything
missing in this PR, please point it out, thank you.
--
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-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1477247017
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6215,6 +6215,71 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+final SqlOperatorFixture f0 = fixture();
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2.0/3, 2)", "DOUBLE NOT NULL",
Review Comment:
Hello, I seem to have forgotten the simplest truth, that is, there are no
fractions in computers, only floating point numbers. I changed 2/3 in the test
to 2.0/3, and it turns out that the result is correct. If 2/3 must output the
correct result, then this bug does not only exist in the log function, but the
pow and abs functions also have similar problems. Regarding log(0) I don't
think it's a bug.
Thank you very much for your guidance on these two PRs of my adaptation
function. I am sorry for causing you a lot of trouble. If there is anything
missing in this PR, please point it out, thank you.
--
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-6223] Add MAP_CONTAINS_KEY function (enabled in Spark library) [calcite]
sonarcloud[bot] commented on PR #3655: URL: https://github.com/apache/calcite/pull/3655#issuecomment-1925638253 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3655) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3655=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3655=false=true) [90.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3655=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3655=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3655) -- 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-6223] Add MAP_CONTAINS_KEY function (enabled in Spark library) [calcite]
caicancai commented on code in PR #3655:
URL: https://github.com/apache/calcite/pull/3655#discussion_r1477205004
##
core/src/main/java/org/apache/calcite/sql/type/NonNullableAccessors.java:
##
@@ -52,4 +52,10 @@ public static RelDataType
getComponentTypeOrThrow(RelDataType type) {
return requireNonNull(type.getComponentType(),
() -> "componentType is null for " + type);
}
+
+ @API(since = "1.37", status = API.Status.EXPERIMENTAL)
Review Comment:
done
##
core/src/main/java/org/apache/calcite/sql/type/MapKeyOperandTypeChecker.java:
##
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.sql.type;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperandCountRange;
+import org.apache.calcite.sql.SqlOperator;
+
+import com.google.common.collect.ImmutableList;
+
+import static
org.apache.calcite.sql.type.NonNullableAccessors.getKeyTypeOrThrow;
+import static org.apache.calcite.util.Static.RESOURCE;
+
+/**
+ * Parameter type-checking strategy where types must be Map and Map key type.
+ */
+public class MapKeyOperandTypeChecker implements SqlOperandTypeChecker {
+ @Override public boolean checkOperandTypes(
+ SqlCallBinding callBinding,
+ boolean throwOnFailure) {
+final SqlNode op0 = callBinding.operand(0);
+if (!OperandTypes.MAP.checkSingleOperandType(
+callBinding,
+op0,
+0,
+throwOnFailure)) {
+ return false;
+}
+
+final RelDataType mapComponentType =
+getKeyTypeOrThrow(SqlTypeUtil.deriveType(callBinding, op0));
+final SqlNode op1 = callBinding.operand(1);
+RelDataType mapType1 = SqlTypeUtil.deriveType(callBinding, op1);
+
+RelDataType biggest =
+callBinding.getTypeFactory().leastRestrictive(
+ImmutableList.of(mapComponentType, mapType1));
+if (biggest == null) {
+ if (throwOnFailure) {
+throw callBinding.newError(
+RESOURCE.typeNotComparable(
+mapComponentType.toString(), mapType1.toString()));
+ }
+
+ return false;
+}
+return true;
+ }
+
+ @Override public SqlOperandCountRange getOperandCountRange() {
+return SqlOperandCountRanges.of(2);
+ }
+
+ @Override public String getAllowedSignatures(SqlOperator op, String opName) {
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-6223] Add MAP_CONTAINS_KEY function (enabled in Spark library) [calcite]
caicancai commented on code in PR #3655:
URL: https://github.com/apache/calcite/pull/3655#discussion_r1477205004
##
core/src/main/java/org/apache/calcite/sql/type/NonNullableAccessors.java:
##
@@ -52,4 +52,10 @@ public static RelDataType
getComponentTypeOrThrow(RelDataType type) {
return requireNonNull(type.getComponentType(),
() -> "componentType is null for " + type);
}
+
+ @API(since = "1.37", status = API.Status.EXPERIMENTAL)
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-6223] Add MAP_CONTAINS_KEY function (enabled in Spark library) [calcite]
caicancai commented on code in PR #3655:
URL: https://github.com/apache/calcite/pull/3655#discussion_r1477203257
##
core/src/main/java/org/apache/calcite/sql/type/MapKeyOperandTypeChecker.java:
##
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.sql.type;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperandCountRange;
+import org.apache.calcite.sql.SqlOperator;
+
+import com.google.common.collect.ImmutableList;
+
+import static
org.apache.calcite.sql.type.NonNullableAccessors.getKeyTypeOrThrow;
+import static org.apache.calcite.util.Static.RESOURCE;
+
+/**
+ * Parameter type-checking strategy where types must be Map and Map key type.
+ */
+public class MapKeyOperandTypeChecker implements SqlOperandTypeChecker {
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
