Re: [PR] [CALCITE-6247] BigQuery FORMAT_DATE function handles incorrectly the %e format specifier [calcite]
sonarcloud[bot] commented on PR #3678: URL: https://github.com/apache/calcite/pull/3678#issuecomment-1935260458 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3678) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3678=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3678=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3678=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3678=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3678) -- 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-6032] Multilevel correlated query is failing in RelDecorrela… [calcite]
HanumathRao commented on PR #3640:
URL: https://github.com/apache/calcite/pull/3640#issuecomment-1935259456
> > The trimUnusedField ,in the prepareSql function call path, is called
conditionally based on the configHolder's isTrimUnusedFields flag and due to
this it results in **trimUnusedFields getting called before decorrelateQuery**
is called.
>
> Maybe I'm missing something, but isn't `decorrelateQuery` called before
`trimUnusedFields` in `prepareSql` ? (so either we trim after decorrelate, or
we don't trim at all, depending on the configHolder)
>
> ```
> if (this.context.config().forceDecorrelate()) {
> // Sub-query decorrelation.
> root = root.withRel(decorrelate(sqlToRelConverter, sqlQuery,
root.rel));
> }
>
> if (configHolder.get().isTrimUnusedFields()) {
> // Trim unused fields.
> root = trimUnusedFields(root);
>
> Hook.TRIMMED.run(root.rel);
> }
> ```
Yeah, your observation is right, and there's a subtle issue that requires
attention in this case.
Before the decorrelate function is invoked, the plan (for the added test in
the PR) appears as follows:
```
LogicalAggregate(group=[{}], INVAMOUNT=[SUM($0)], INVCOUNT=[COUNT()])
LogicalProject($f0=[*($2, $SCALAR_QUERY({
LogicalAggregate(group=[{}], EXPR$0=[MAX($0)])
LogicalProject($f0=[+($0, $1)])
LogicalFilter(condition=[AND(IS NOT NULL($2), IS NOT NULL($3), =($4,
$cor0.C5633_503), =($5, $SCALAR_QUERY({
LogicalAggregate(group=[{}], EXPR$0=[MAX($0)])
LogicalProject(C5633_2186=[$5])
LogicalFilter(condition=[AND(=($4, $cor1.C5633_2187), =($2,
$cor1.C5633_2184), =($3, $cor1.C5633_2185), <=($5, $cor0.C5633_485))])
JdbcTableScan(table=[[BASEJDBC, T1704]])
})))], variablesSet=[[$cor1]])
JdbcTableScan(table=[[BASEJDBC, T1704]])
}))])
LogicalFilter(condition=[AND(<($2, 10), >($2, 0))])
JdbcTableScan(table=[[BASEJDBC, INVOICE]])
```
The decorrelate function before the trimFields operation is essentially a
no-op. This is because decorrelation only occurs when a Correlate node is
created, which is determined by a triggering check.
```
if (!corelMap.hasCorrelation()) {
return rootRel;
}
```
The hasCorrelation function returns true only if mapCorToCorRel is nonEmpty.
However, in this scenario, mapCorToCorRel is empty because no Correlate node
exists. Therefore, the decorrelate function essentially does nothing.
However, after the trimUnusedFields operation is executed on the tree, it
prunes certain fields that are essential for correlated variables.
Consequently, a new node, LogicalProject(C5633_505=[$2]), is introduced.
```
LogicalAggregate(group=[{}], INVAMOUNT=[SUM($0)], INVCOUNT=[COUNT()])
LogicalProject($f0=[*($0, $SCALAR_QUERY({
LogicalAggregate(group=[{}], EXPR$0=[MAX($0)])
LogicalProject($f0=[+($0, $1)])
LogicalFilter(condition=[AND(IS NOT NULL($2), IS NOT NULL($3), =($4,
$cor0.C5633_503), =($5, $SCALAR_QUERY({
LogicalAggregate(group=[{}], EXPR$0=[MAX($0)])
LogicalProject(C5633_2186=[$5])
LogicalFilter(condition=[AND(=($4, $cor1.C5633_2187), =($2,
$cor1.C5633_2184), =($3, $cor1.C5633_2185), <=($5, $cor0.C5633_485))])
JdbcTableScan(table=[[BASEJDBC, T1704]])
})))], variablesSet=[[$cor1]])
JdbcTableScan(table=[[BASEJDBC, T1704]])
}))])
LogicalFilter(condition=[SEARCH($0, Sarg[(0..10)])])
LogicalProject(C5633_505=[$2]). --- a new node introduced by
trimUnusedFields
JdbcTableScan(table=[[BASEJDBC, INVOICE]])
```
The subsequent optimization of this tree leads to an exception because the
tree is no longer correct.
This issue is specific to the prepareSql path and is not observed during the
plannerImpl rel path.
[link](https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java#L265).
Two potential solutions have been identified:
1. Fix the RelTrimmer to avoid generating the problematic node.
2. Ensure consistency between both code paths by refraining from calling the
RelTrimmer in the prepareSql path.
I chose the second option, as it seems logical to call the RelTrimmer only
after the SubQueryRemovalRule has been applied.
Please let me know if it is clear and thank you for reviewing the changes.
--
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-6252] BigQuery FORMAT_DATE uses the wrong calendar for Julian dates [calcite]
sonarcloud[bot] commented on PR #3680: URL: https://github.com/apache/calcite/pull/3680#issuecomment-1935250669 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3680) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3680=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3680=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3680=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3680=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3680) -- 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-6252] BigQuery FORMAT_DATE uses the wrong calendar for Julian dates [calcite]
mihaibudiu opened a new pull request, #3680: URL: https://github.com/apache/calcite/pull/3680 This was much trickier to fix than I expected. I think that it's because of some bugs in the Java Calendar/SimpleDateFormatter implementation. Using the LocalDate class provides correct results. I have also changed the return type of FORMAT_DATE from VARCHAR(2000) to VARCHAR, because it can really return results longer than 2000 characters, if the format string is long. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(calcite) branch main updated (80f96bbcb5 -> b64dcff73d)
This is an automated email from the ASF dual-hosted git repository. jhyde pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git omit 80f96bbcb5 [CALCITE-6256] Correct incorrect rendering of HTML on InnoDB adapter page omit 7f0f4f05b9 [CALCITE-6256] Fix incorrect rendering of HTML on InnoDB adapter page new b64dcff73d [CALCITE-6256] Incorrect rendering of HTML on InnoDB adapter page This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (80f96bbcb5) \ N -- N -- N refs/heads/main (b64dcff73d) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes:
(calcite) 01/01: [CALCITE-6256] Incorrect rendering of HTML on InnoDB adapter page
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit b64dcff73db17e492f060b92d7581a90763cc9c2
Author: Zhengqiang Duan
AuthorDate: Fri Feb 9 07:29:58 2024 +0800
[CALCITE-6256] Incorrect rendering of HTML on InnoDB adapter page
Close apache/calcite#3679
---
site/_docs/innodb_adapter.md | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/site/_docs/innodb_adapter.md b/site/_docs/innodb_adapter.md
index 58ef596526..6d5c760410 100644
--- a/site/_docs/innodb_adapter.md
+++ b/site/_docs/innodb_adapter.md
@@ -96,8 +96,11 @@ a MySQL "scott" database:
{% endhighlight %}
`sqlFilePath` is a list of DDL files, you can generate table
-definitions by executing `mysqldump -d -u -p -h
- ` in command-line.
+definitions by executing `mysqldump` in command-line:
+
+{% highlight bash %}
+mysqldump -d -u -p -h
+{% endhighlight %}
The file content of `/path/scott.sql` is as follows:
Re: [PR] [CALCITE-6247] BigQuery FORMAT_DATE function handles incorrectly the %e format specifier [calcite]
Anthrino commented on code in PR #3678:
URL: https://github.com/apache/calcite/pull/3678#discussion_r1483769028
##
core/src/main/java/org/apache/calcite/util/format/FormatElementEnum.java:
##
@@ -81,6 +81,13 @@ public enum FormatElementEnum implements FormatElement {
sb.append(work.eeeFormat.format(date));
}
},
+ E("d", "The day of the month as a decimal number (01-31) left-padded with
space") {
Review Comment:
Hi @mihaibudiu, all changes looks good to me.
We can replace `(01-31)` in the comment to `(1-31)`, and maybe mention
"single digits left-padded with space" to avoid confusion and align with BQ
docs.
--
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) 02/02: [CALCITE-6256] Correct incorrect rendering of HTML on InnoDB adapter page
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 80f96bbcb5d75421df5e005bdc67c165f451e56d
Author: Zhengqiang Duan
AuthorDate: Fri Feb 9 08:41:31 2024 +0800
[CALCITE-6256] Correct incorrect rendering of HTML on InnoDB adapter page
---
site/_docs/innodb_adapter.md | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/site/_docs/innodb_adapter.md b/site/_docs/innodb_adapter.md
index 1395d7d43e..6d5c760410 100644
--- a/site/_docs/innodb_adapter.md
+++ b/site/_docs/innodb_adapter.md
@@ -95,8 +95,12 @@ a MySQL "scott" database:
}
{% endhighlight %}
-`sqlFilePath` is a list of DDL files, you can generate table definitions by
executing
-`mysqldump -d -u -p -h ` in
command-line.
+`sqlFilePath` is a list of DDL files, you can generate table
+definitions by executing `mysqldump` in command-line:
+
+{% highlight bash %}
+mysqldump -d -u -p -h
+{% endhighlight %}
The file content of `/path/scott.sql` is as follows:
(calcite) 01/02: [CALCITE-6256] Fix incorrect rendering of HTML on InnoDB adapter page
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 7f0f4f05b9f921706f3c3d9000a87f9fbeda0025
Author: Zhengqiang Duan
AuthorDate: Fri Feb 9 07:29:58 2024 +0800
[CALCITE-6256] Fix incorrect rendering of HTML on InnoDB adapter page
---
site/_docs/innodb_adapter.md | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/site/_docs/innodb_adapter.md b/site/_docs/innodb_adapter.md
index 58ef596526..1395d7d43e 100644
--- a/site/_docs/innodb_adapter.md
+++ b/site/_docs/innodb_adapter.md
@@ -95,9 +95,8 @@ a MySQL "scott" database:
}
{% endhighlight %}
-`sqlFilePath` is a list of DDL files, you can generate table
-definitions by executing `mysqldump -d -u -p -h
- ` in command-line.
+`sqlFilePath` is a list of DDL files, you can generate table definitions by
executing
+`mysqldump -d -u -p -h ` in
command-line.
The file content of `/path/scott.sql` is as follows:
(calcite) branch main updated (2e384ed7d0 -> 80f96bbcb5)
This is an automated email from the ASF dual-hosted git repository. jhyde pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git from 2e384ed7d0 [CALCITE-6238] Exception while evaluating ROUND/TRUNCATE functions new 7f0f4f05b9 [CALCITE-6256] Fix incorrect rendering of HTML on InnoDB adapter page new 80f96bbcb5 [CALCITE-6256] Correct incorrect rendering of HTML on InnoDB adapter page The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: site/_docs/innodb_adapter.md | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-)
Re: [PR] [CALCITE-6256] Incorrect rendering of HTML on InnoDB adapter page [calcite]
julianhyde merged PR #3679: URL: https://github.com/apache/calcite/pull/3679 -- 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-6256] Correct incorrect rendering of HTML on InnoDB adapter page [calcite]
strongduanmu commented on PR #3679: URL: https://github.com/apache/calcite/pull/3679#issuecomment-1935161419 Thank you very much for your guidance. I will pay attention to these norms of the Calcite community so that my next contribution can be more perfect. -- 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-6256] Correct incorrect rendering of HTML on InnoDB adapter page [calcite]
julianhyde commented on PR #3679: URL: https://github.com/apache/calcite/pull/3679#issuecomment-1935156321 You changed 'Fix' to 'Correct' in the commit message. Instead, can you change the commit message to the jira case summary? (If a commit message describes a bug, it is implied that the commit fixes the bug.) -- 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-6256] Fix incorrect rendering of HTML on InnoDB adapter page [calcite]
strongduanmu commented on PR #3679: URL: https://github.com/apache/calcite/pull/3679#issuecomment-1935143164 @julianhyde Thank you for your advice, I will modify this pr. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6247] BigQuery FORMAT_DATE function handles incorrectly the %e format specifier [calcite]
sonarcloud[bot] commented on PR #3678: URL: https://github.com/apache/calcite/pull/3678#issuecomment-1935121249 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3678) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3678=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3678=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3678=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3678=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3678) -- 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-6256] Fix incorrect rendering of HTML on InnoDB adapter page [calcite]
julianhyde commented on PR #3679: URL: https://github.com/apache/calcite/pull/3679#issuecomment-1935120464 Seems to fix the problem. I think that the "`mysqldump -d -u -p -h `" line would be better as its own code box, rather than inline; can you change it to use the highlight markup? We don't allow commit messages with the word 'fix'. Can you change the commit message to match the jira case summary. -- 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 (f837ffa877 -> 2e384ed7d0)
This is an automated email from the ASF dual-hosted git repository. mbudiu pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git from f837ffa877 [CALCITE-6228] ELEMENT function infers incorrect return type add 2e384ed7d0 [CALCITE-6238] Exception while evaluating ROUND/TRUNCATE functions No new revisions were added by this update. Summary of changes: .../calcite/sql/fun/SqlStdOperatorTable.java | 8 ++-- .../org/apache/calcite/sql/type/OperandTypes.java | 19 .../org/apache/calcite/test/SqlOperatorTest.java | 50 +- 3 files changed, 63 insertions(+), 14 deletions(-)
Re: [PR] [CALCITE-6238] Exception while evaluating ROUND/TRUNCATE functions [calcite]
mihaibudiu merged PR #3672: URL: https://github.com/apache/calcite/pull/3672 -- 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-6256] Fix incorrect rendering of HTML on InnoDB adapter page [calcite]
strongduanmu opened a new pull request, #3679: URL: https://github.com/apache/calcite/pull/3679 The new rendered HTML is as follows(IDEA Local preview without CSS style). https://github.com/apache/calcite/assets/10829171/441fff2e-2c51-421a-862b-8f98cd9fb5ad;> -- 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-6247] BigQuery FORMAT_DATE function handles incorrectly the %e format specifier [calcite]
mihaibudiu opened a new pull request, #3678: URL: https://github.com/apache/calcite/pull/3678 (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-2980] Implement the FORMAT clause of the CAST operator [calcite]
julianhyde commented on code in PR #3677: URL: https://github.com/apache/calcite/pull/3677#discussion_r1483662654 ## core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java: ## @@ -684,7 +686,13 @@ protected RexNode convertCast( final boolean safe = kind == SqlKind.SAFE_CAST; final SqlNode left = call.operand(0); final SqlNode right = call.operand(1); +final SqlNode format = call.getOperandList().size() > 2 ? +call.operand(2) : null; + final RexBuilder rexBuilder = cx.getRexBuilder(); +final RexNode arg = cx.convertExpression(left); +final RexNode formatArg = isNull(format) ? null : cx.convertExpression(format); Review Comment: Why use the `isNull` method, here and elsewhere? `isNull` detects the `NULL` literal, but I don't think that `CAST(d AS DATE FORMAT NULL)` is even allowed (correct me if I'm wrong). The only null is a java null, where you pass a RexLiteral whose value is null if there was no `FORMAT` clause. I am wondering whether you intend to support an expression for the format clause, e.g. `CAST(d AS DATE FORMAT '-' || 'mm')` but as there are no tests I can't answer that question. -- 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-2980] Implement the FORMAT clause of the CAST operator [calcite]
Anthrino opened a new pull request, #3677: URL: https://github.com/apache/calcite/pull/3677 (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-6238] Exception while evaluating ROUND/TRUNCATE functions [calcite]
mihaibudiu commented on PR #3672: URL: https://github.com/apache/calcite/pull/3672#issuecomment-1934970229 I have squashed and rebased, in case anyone wants to click the "approve" button. Thank you for the reviews. -- 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-6238] Exception while evaluating ROUND/TRUNCATE functions [calcite]
sonarcloud[bot] commented on PR #3672: URL: https://github.com/apache/calcite/pull/3672#issuecomment-1934966211 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3672) **Quality Gate passed** Issues  [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite=3672=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3672=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3672=new_coverage=list)  [9.5% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3672=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3672) -- 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-6250] Add hints with restrictions to MongoDB adapter documentation [calcite]
mihaibudiu commented on PR #3676: URL: https://github.com/apache/calcite/pull/3676#issuecomment-1934840822 The titles match, but they do not match the guidelines, which require the title to describe a *problem*, and not the solution to the problem. I personally don't have a problem with the title, but I think that it's good to follow the rules set up by the project. A problem-like title would be "Limitations of MongoDB adapter are not documented" -- 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-1934535856 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3668) **Quality Gate passed** Issues  [4 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)  [59.9% 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-6032] Multilevel correlated query is failing in RelDecorrela… [calcite]
rubenada commented on PR #3640:
URL: https://github.com/apache/calcite/pull/3640#issuecomment-1934283981
> The trimUnusedField ,in the prepareSql function call path, is called
conditionally based on the configHolder's isTrimUnusedFields flag and due to
this it results in **trimUnusedFields getting called before decorrelateQuery**
is called.
Maybe I'm missing something, but isn't `decorrelateQuery` called before
`trimUnusedFields` in `prepareSql` ? (so either we trim after decorrelate, or
we don't trim at all, depending on the configHolder)
```
if (this.context.config().forceDecorrelate()) {
// Sub-query decorrelation.
root = root.withRel(decorrelate(sqlToRelConverter, sqlQuery,
root.rel));
}
if (configHolder.get().isTrimUnusedFields()) {
// Trim unused fields.
root = trimUnusedFields(root);
Hook.TRIMMED.run(root.rel);
}
```
--
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-1934228556 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3668) **Quality Gate passed** Issues  [4 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)  [59.9% 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] Upgrade Cassandra to 4.1.3 and DataStax driver for Cassandra to 4.17.0 [calcite]
asolimando commented on PR #3670: URL: https://github.com/apache/calcite/pull/3670#issuecomment-1933717003 @JiajunBernoulli, do you have any other comments for this PR? I plan to merge it sometime this weekend if there are no concerns. -- 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-6250] Add hints with restrictions to MongoDB adapter documentation [calcite]
TheOezzi commented on PR #3676: URL: https://github.com/apache/calcite/pull/3676#issuecomment-1933653216 Hey ! Okay, I've changed the issue name to match the commit message -- 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]
rubenada commented on PR #3674:
URL: https://github.com/apache/calcite/pull/3674#issuecomment-1933577073
@mihaibudiu this change is subtle, but it has an impact (although we do not
see it in Calcite tests).
By default, this change will have zero effect, because Calcite's default
implementation of `mq.getRowCount(rel)` simply calls
`rel.estimateRowCount(mq)`; so it seems we are getting to the same place with
one extra step; and indeed we are (that's the reason why we do not see any
impact on our tests).
However, the fact of using `mq.getRowCount(rel)` allows downstream projects
to plug their own metadata rowCount and "override" the default rowCount
computation proposed by Calcite. If we call directly
`rel.estimateRowCount(mq)`, this flexibility is lost. In fact, this PR is just
applying our own Calcite rules, see `RelNode#estimateRowCount` javadoc:
```
Don't call this method directly. Instead, use
{@link RelMetadataQuery#getRowCount}, which gives plugins a chance to
override the rel's default ideas about row count.
```
So all these calls to `rel.estimateRowCount(mq)` should have been
`mq.getRowCount(rel)` from the beginning (and we are likely dealing with a
copy-paste of sub-optimal code propagated). As I said before, at that time this
might have been difficult to catch, because by default the effect is the same.
@tanclary @JiajunBernoulli Writing tests for these changes is possible, but
cumbersome. See e.g. the recent commit
https://github.com/apache/calcite/commit/f7069cc5245c22f816c565669f52b4f30b046f4d
which performed a similar change in one location, and how "artificial" it was
to show the impact on a unit test.
Personally, I understand the logic behind this change, and bearing in mind
that is not straightforward to provide tests for this, I'd consider
exceptionally merging the PR without tests. But if others consider that this is
not acceptable, I'll accept that decision.
--
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-6238] Exception while evaluating ROUND/TRUNCATE functions [calcite]
rubenada commented on PR #3672: URL: https://github.com/apache/calcite/pull/3672#issuecomment-1933554417 LGTM -- 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
