You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "mihaibudiu (via GitHub)" <gi...@apache.org> on 2024/10/24 05:47:28 UTC
[PR] [CALCITE-6641] Compiling programs with ASOF joins can report obscure errors [calcite]
mihaibudiu opened a new pull request, #4015:
URL: https://github.com/apache/calcite/pull/4015
(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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6641] Compiling programs with ASOF joins can report obscure errors [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4015:
URL: https://github.com/apache/calcite/pull/4015#discussion_r1815439382
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3911,14 +3918,21 @@ void checkAnd(SqlCall call) {
}
}
- @Override public @Nullable SqlNode visit(final org.apache.calcite.sql.SqlCall call) {
+ @Override public @Nullable SqlNode visit(final SqlCall call) {
SqlKind kind = call.getKind();
- if (kind != SqlKind.AND && kind != SqlKind.EQUALS) {
+ if (kind != SqlKind.AND && kind != SqlKind.EQUALS && kind != SqlKind.CAST) {
Review Comment:
The JavaDoc says "shuttle which determines whether an expression is a simple conjunction of equalities". AND is conjunction, and EQUALS is equalities. CAST is only allowed when applied to field names.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6641] Compiling programs with ASOF joins can report obscure errors [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4015:
URL: https://github.com/apache/calcite/pull/4015#discussion_r1815842243
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3911,14 +3918,21 @@ void checkAnd(SqlCall call) {
}
}
- @Override public @Nullable SqlNode visit(final org.apache.calcite.sql.SqlCall call) {
+ @Override public @Nullable SqlNode visit(final SqlCall call) {
SqlKind kind = call.getKind();
- if (kind != SqlKind.AND && kind != SqlKind.EQUALS) {
+ if (kind != SqlKind.AND && kind != SqlKind.EQUALS && kind != SqlKind.CAST) {
Review Comment:
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6641] Compiling programs with ASOF joins can report obscure errors [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4015:
URL: https://github.com/apache/calcite/pull/4015#issuecomment-2439023438
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4015) **Quality Gate passed**
Issues
 [7 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4015&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4015&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4015&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [96.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4015&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4015&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4015)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6641] Compiling programs with ASOF joins can report obscure errors [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4015:
URL: https://github.com/apache/calcite/pull/4015#issuecomment-2438778390
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4015) **Quality Gate passed**
Issues
 [7 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4015&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4015&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4015&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [96.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4015&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4015&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4015)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6641] Compiling programs with ASOF joins can report obscure errors [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #4015:
URL: https://github.com/apache/calcite/pull/4015#discussion_r1815181924
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3911,14 +3918,21 @@ void checkAnd(SqlCall call) {
}
}
- @Override public @Nullable SqlNode visit(final org.apache.calcite.sql.SqlCall call) {
+ @Override public @Nullable SqlNode visit(final SqlCall call) {
SqlKind kind = call.getKind();
- if (kind != SqlKind.AND && kind != SqlKind.EQUALS) {
+ if (kind != SqlKind.AND && kind != SqlKind.EQUALS && kind != SqlKind.CAST) {
Review Comment:
It would be nice if you could explain why SqlKind cannot be equal to these types
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6641] Compiling programs with ASOF joins can report obscure errors [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4015:
URL: https://github.com/apache/calcite/pull/4015#issuecomment-2434369600
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4015) **Quality Gate passed**
Issues
 [7 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4015&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4015&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4015&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [95.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4015&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4015&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4015)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6641] Compiling programs with ASOF joins can report obscure errors [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #4015:
URL: https://github.com/apache/calcite/pull/4015#discussion_r1815860062
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3911,14 +3918,21 @@ void checkAnd(SqlCall call) {
}
}
- @Override public @Nullable SqlNode visit(final org.apache.calcite.sql.SqlCall call) {
+ @Override public @Nullable SqlNode visit(final SqlCall call) {
SqlKind kind = call.getKind();
- if (kind != SqlKind.AND && kind != SqlKind.EQUALS) {
+ if (kind != SqlKind.AND && kind != SqlKind.EQUALS && kind != SqlKind.CAST) {
illegal = true;
Review Comment:
I check the `checkAnd(call)` and this method, Maybe we can exit directly when we find the illegal value is true.
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3911,14 +3918,21 @@ void checkAnd(SqlCall call) {
}
}
- @Override public @Nullable SqlNode visit(final org.apache.calcite.sql.SqlCall call) {
+ @Override public @Nullable SqlNode visit(final SqlCall call) {
SqlKind kind = call.getKind();
- if (kind != SqlKind.AND && kind != SqlKind.EQUALS) {
+ if (kind != SqlKind.AND && kind != SqlKind.EQUALS && kind != SqlKind.CAST) {
illegal = true;
Review Comment:
Some minor advice:
1) SqlValidatorImpl#validateJoin(Line#3784). There is an extra `{}` behind the ASOF.
2) ConjunctionOfEqualities#checkAnd method's java comment. Before `Children` There is an extra space.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6641] Compiling programs with ASOF joins can report obscure errors [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4015:
URL: https://github.com/apache/calcite/pull/4015#discussion_r1817313324
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3911,14 +3918,21 @@ void checkAnd(SqlCall call) {
}
}
- @Override public @Nullable SqlNode visit(final org.apache.calcite.sql.SqlCall call) {
+ @Override public @Nullable SqlNode visit(final SqlCall call) {
SqlKind kind = call.getKind();
- if (kind != SqlKind.AND && kind != SqlKind.EQUALS) {
+ if (kind != SqlKind.AND && kind != SqlKind.EQUALS && kind != SqlKind.CAST) {
illegal = true;
Review Comment:
the extra {} are actually useful, they create a local scope, and the variables in the local scope do not interfere with variables from the rest of the function. It also prevents accidental updates to other variables that may be in scope. If also helps with the debugger too: such variables go out of scope when the inner scope {} terminates, so in large functions with lots of variables it makes the debugging experience better. I use this trick a lot when I have to write very large 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6641] Compiling programs with ASOF joins can report obscure errors [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu merged PR #4015:
URL: https://github.com/apache/calcite/pull/4015
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org