Re: [PR] [CALCITE-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-21 Thread via GitHub


mihaibudiu merged PR #3435:
URL: https://github.com/apache/calcite/pull/3435


-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-21 Thread via GitHub


sonarcloud[bot] commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-2013217304

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_calcite=3435) 
**Quality Gate passed**  
   Issues  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [10 New 
issues](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png
 '') [0 Accepted 
issues](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_accepted_issues=list)
   
   Measures  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3435=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [98.4% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_coverage=list)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0.0% Duplication on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_duplicated_lines_density=list)
  
 
   [See analysis details on 
SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3435)
   
   


-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-21 Thread via GitHub


mihaibudiu commented on code in PR #3435:
URL: https://github.com/apache/calcite/pull/3435#discussion_r1534301224


##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -10862,6 +10810,47 @@ private static void 
checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
 "3", "BIGINT NOT NULL");
   }
 
+  @Test void testExtractTime() {
+final SqlOperatorFixture f = fixture();
+f.setFor(SqlStdOperatorTable.EXTRACT, VM_FENNEL, VM_JAVA);
+
+final String fail = "Cannot apply 'EXTRACT' to arguments of type 
'EXTRACT\\(<.*> "
++ "FROM \\)'\\. "
++ "Supported form\\(s\\): .*\\n.*\\n.*";
+
+f.checkFails("extract(^a^ from time '12:34:56')",
+"'A' is not a valid time frame", false);
+f.checkFails("^extract(epoch from time '12:34:56')^",
+fail, false);
+
+f.checkScalar("extract(second from time '12:34:56')",
+"56", "BIGINT NOT NULL");
+f.checkScalar("extract(millisecond from time '12:34:56')",
+"56000", "BIGINT NOT NULL");
+f.checkScalar("extract(microsecond from time '12:34:56')",
+"5600", "BIGINT NOT NULL");
+f.checkScalar("extract(nanosecond from time '12:34:56')",
+"560", "BIGINT NOT NULL");
+f.checkScalar("extract(minute from time '12:34:56')",
+"34", "BIGINT NOT NULL");
+f.checkScalar("extract(hour from time '12:34:56')",
+"12", "BIGINT NOT NULL");
+f.checkFails("^extract(day from time '12:34:56')^", fail, false);
+f.checkFails("^extract(month from time '12:34:56')^", fail, false);
+f.checkFails("^extract(quarter from time '12:34:56')^", fail, false);
+f.checkFails("^extract(year from time '12:34:56')^", fail, false);
+f.checkFails("^extract(isoyear from time '12:34:56')^", fail, false);
+f.checkFails("^extract(doy from time '12:34:56')^", fail, false);
+f.checkFails("^extract(dow from time '12:34:56')^", fail, false);
+f.checkFails("^extract(week from time '12:34:56')^", fail, false);
+f.checkFails("^extract(decade from time '12:34:56')^", fail, false);
+f.checkFails("^extract(century from time '12:34:56')^", fail, false);
+f.checkFails("^extract(century from time '12:34:56')^", fail, false);
+f.checkFails("^extract(century from time '12:34:56')^", fail, false);

Review Comment:
   noticed some duplicated tests, I will delete these before merging.



-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-21 Thread via GitHub


rubenada commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-2011373019

   Thanks @mihaibudiu !
   @snuyanzin  do you have any further remark? Otherwise IMO this PR is in a 
good shape to be merged.


-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-19 Thread via GitHub


sonarcloud[bot] commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-2008561216

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_calcite=3435) 
**Quality Gate passed**  
   Issues  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [11 New 
issues](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png
 '') [0 Accepted 
issues](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_accepted_issues=list)
   
   Measures  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3435=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [98.4% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_coverage=list)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0.0% Duplication on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_duplicated_lines_density=list)
  
 
   [See analysis details on 
SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3435)
   
   


-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-19 Thread via GitHub


mihaibudiu commented on code in PR #3435:
URL: https://github.com/apache/calcite/pull/3435#discussion_r1531432890


##
site/_docs/history.md:
##
@@ -57,6 +57,10 @@ other software versions as specified in gradle.properties.
  Bug-fixes, API changes and minor enhancements
 {: #fixes-1-37-0}
 
+* In the context of [CALCITE-6015] the visibility of the method

Review Comment:
   sorry about that, I have fixed this, and also squashed the commits



-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-19 Thread via GitHub


rubenada commented on code in PR #3435:
URL: https://github.com/apache/calcite/pull/3435#discussion_r1530145137


##
site/_docs/history.md:
##
@@ -57,6 +57,10 @@ other software versions as specified in gradle.properties.
  Bug-fixes, API changes and minor enhancements
 {: #fixes-1-37-0}
 
+* In the context of [CALCITE-6015] the visibility of the method

Review Comment:
   @mihaibudiu I just noticed, this comment should better be placed a few lines 
up, right below  the 
   ```
    Breaking Changes
   {: #breaking-1-37-0}
   ```



-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-09 Thread via GitHub


sonarcloud[bot] commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-1986965751

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_calcite=3435) 
**Quality Gate passed**  
   Issues  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [12 New 
issues](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png
 '') [0 Accepted 
issues](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_accepted_issues=list)
   
   Measures  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3435=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [98.4% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_coverage=list)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0.0% Duplication on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_duplicated_lines_density=list)
  
 
   [See analysis details on 
SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3435)
   
   


-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-09 Thread via GitHub


mihaibudiu commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-1986961743

   Squashed the commits in preparation for merging.


-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-09 Thread via GitHub


rubenada commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-1986958025

   Thanks @mihaibudiu , no further comment from my side. 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



Re: [PR] [CALCITE-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-09 Thread via GitHub


sonarcloud[bot] commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-1986956048

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_calcite=3435) 
**Quality Gate passed**  
   Issues  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [12 New 
issues](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png
 '') [0 Accepted 
issues](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_accepted_issues=list)
   
   Measures  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3435=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [98.4% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_coverage=list)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0.0% Duplication on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_duplicated_lines_density=list)
  
 
   [See analysis details on 
SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3435)
   
   


-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-09 Thread via GitHub


mihaibudiu commented on code in PR #3435:
URL: https://github.com/apache/calcite/pull/3435#discussion_r1518646046


##
site/_docs/history.md:
##
@@ -57,6 +57,9 @@ other software versions as specified in gradle.properties.
  Bug-fixes, API changes and minor enhancements
 {: #fixes-1-37-0}
 
+* The visibility of the method SqlCall.getCallSignature has been converted

Review Comment:
   @rubenada changed List to Set and updated the history file.



-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-09 Thread via GitHub


rubenada commented on code in PR #3435:
URL: https://github.com/apache/calcite/pull/3435#discussion_r1518540130


##
site/_docs/history.md:
##
@@ -57,6 +57,9 @@ other software versions as specified in gradle.properties.
  Bug-fixes, API changes and minor enhancements
 {: #fixes-1-37-0}
 
+* The visibility of the method SqlCall.getCallSignature has been converted

Review Comment:
   Thanks for including this. I'd suggest to give a bit more info, e.g. 
something a long the lines:
   
   In the context of  [https://issues.apache.org/jira/browse/CALCITE-6015;>CALCITE-6015] the 
visibility of the method `SqlCall.getCallSignature` has been converted from 
`protected` to `public`, so any subclass overriding it might need to be 
adjusted accordingly.
   



-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-08 Thread via GitHub


sonarcloud[bot] commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-1986578410

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_calcite=3435) 
**Quality Gate passed**  
   Issues  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [12 New 
issues](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png
 '') [0 Accepted 
issues](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_accepted_issues=list)
   
   Measures  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3435=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [98.4% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_coverage=list)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0.0% Duplication on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_duplicated_lines_density=list)
  
 
   [See analysis details on 
SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3435)
   
   


-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-08 Thread via GitHub


mihaibudiu commented on code in PR #3435:
URL: https://github.com/apache/calcite/pull/3435#discussion_r1518410698


##
core/src/main/java/org/apache/calcite/sql/fun/SqlExtractFunction.java:
##
@@ -83,8 +88,102 @@ public SqlExtractFunction(String name) {
 //startUnit = EPOCH and timeFrameName = 'MINUTE15'.
 //
 // If the latter, check that timeFrameName is valid.
-validator.validateTimeFrame(
-(SqlIntervalQualifier) call.getOperandList().get(0));
+SqlIntervalQualifier qualifier = call.operand(0);
+validator.validateTimeFrame(qualifier);
+TimeUnitRange range = qualifier.timeUnitRange;
+
+RelDataType type = validator.getValidatedNodeTypeIfKnown(call.operand(1));
+if (type == null) {
+  return;
+}
+
+SqlTypeName typeName = type.getSqlTypeName();
+boolean legal;
+switch (range) {
+case YEAR:
+case MONTH:
+case ISOYEAR:
+case QUARTER:
+case DECADE:
+case CENTURY:
+case MILLENNIUM:
+  legal = new ImmutableList.Builder()
+  .add(SqlTypeName.DATE)
+  .add(SqlTypeName.TIMESTAMP)
+  .add(SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE)
+  .addAll(SqlTypeName.YEAR_INTERVAL_TYPES)
+  .build()
+  .contains(typeName);
+  break;
+case WEEK:
+case DOW:
+case ISODOW:
+case DOY:
+  legal = new ImmutableList.Builder()

Review Comment:
   I am now using static fields for these constant lists.



-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-08 Thread via GitHub


sonarcloud[bot] commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-1986428813

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_calcite=3435) 
**Quality Gate passed**  
   Issues  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [12 New 
issues](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png
 '') [0 Accepted 
issues](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_accepted_issues=list)
   
   Measures  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3435=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [98.4% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_coverage=list)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0.0% Duplication on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_duplicated_lines_density=list)
  
 
   [See analysis details on 
SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3435)
   
   


-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-08 Thread via GitHub


mihaibudiu commented on code in PR #3435:
URL: https://github.com/apache/calcite/pull/3435#discussion_r1518271876


##
core/src/main/java/org/apache/calcite/sql/SqlCall.java:
##
@@ -191,7 +191,7 @@ public int operandCount() {
* Returns a string describing the actual argument types of a call, e.g.
* "SUBSTR(VARCHAR(12), NUMBER(3,2), INTEGER)".
*/
-  protected String getCallSignature(
+  public String getCallSignature(

Review Comment:
   I have added this to history.md. Thank you for pointing this out.



-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-08 Thread via GitHub


rubenada commented on code in PR #3435:
URL: https://github.com/apache/calcite/pull/3435#discussion_r1518007495


##
core/src/main/java/org/apache/calcite/sql/SqlCall.java:
##
@@ -191,7 +191,7 @@ public int operandCount() {
* Returns a string describing the actual argument types of a call, e.g.
* "SUBSTR(VARCHAR(12), NUMBER(3,2), INTEGER)".
*/
-  protected String getCallSignature(
+  public String getCallSignature(

Review Comment:
   Usually in these cases, to make sure the Release Manager won't forget about 
it, what we do is editing the `history.md`: in that file, at the top, there is 
the "skeleton" with the sections of the next release (commented out), you could 
add a point in there in the "breaking changes" section mentioning this.



-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-08 Thread via GitHub


mihaibudiu commented on code in PR #3435:
URL: https://github.com/apache/calcite/pull/3435#discussion_r1517996592


##
core/src/main/java/org/apache/calcite/sql/SqlCall.java:
##
@@ -191,7 +191,7 @@ public int operandCount() {
* Returns a string describing the actual argument types of a call, e.g.
* "SUBSTR(VARCHAR(12), NUMBER(3,2), INTEGER)".
*/
-  protected String getCallSignature(
+  public String getCallSignature(

Review Comment:
   what should I do about 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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-08 Thread via GitHub


rubenada commented on code in PR #3435:
URL: https://github.com/apache/calcite/pull/3435#discussion_r1517372282


##
core/src/main/java/org/apache/calcite/sql/SqlCall.java:
##
@@ -191,7 +191,7 @@ public int operandCount() {
* Returns a string describing the actual argument types of a call, e.g.
* "SUBSTR(VARCHAR(12), NUMBER(3,2), INTEGER)".
*/
-  protected String getCallSignature(
+  public String getCallSignature(

Review Comment:
   This change on method visibility, although minor, should be considered a 
breaking change and IMO mentioned briefly on the RN: any downstream project 
that had overridden it as `protected` would have a compilation issue with the 
new version, and would need to adjust their code to override it as `public`.



-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-03-05 Thread via GitHub


snuyanzin commented on code in PR #3435:
URL: https://github.com/apache/calcite/pull/3435#discussion_r1512708973


##
core/src/main/java/org/apache/calcite/sql/fun/SqlExtractFunction.java:
##
@@ -83,8 +88,102 @@ public SqlExtractFunction(String name) {
 //startUnit = EPOCH and timeFrameName = 'MINUTE15'.
 //
 // If the latter, check that timeFrameName is valid.
-validator.validateTimeFrame(
-(SqlIntervalQualifier) call.getOperandList().get(0));
+SqlIntervalQualifier qualifier = call.operand(0);
+validator.validateTimeFrame(qualifier);
+TimeUnitRange range = qualifier.timeUnitRange;
+
+RelDataType type = validator.getValidatedNodeTypeIfKnown(call.operand(1));
+if (type == null) {
+  return;
+}
+
+SqlTypeName typeName = type.getSqlTypeName();
+boolean legal;
+switch (range) {
+case YEAR:
+case MONTH:
+case ISOYEAR:
+case QUARTER:
+case DECADE:
+case CENTURY:
+case MILLENNIUM:
+  legal = new ImmutableList.Builder()
+  .add(SqlTypeName.DATE)
+  .add(SqlTypeName.TIMESTAMP)
+  .add(SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE)
+  .addAll(SqlTypeName.YEAR_INTERVAL_TYPES)
+  .build()
+  .contains(typeName);
+  break;
+case WEEK:
+case DOW:
+case ISODOW:
+case DOY:
+  legal = new ImmutableList.Builder()

Review Comment:
   should we extract the result of the builder here and others into constants 
(may be sets instead of lists) to avoid building it each time we come here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] [CALCITE-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-02-26 Thread via GitHub


sonarcloud[bot] commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-1965666034

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_calcite=3435) 
**Quality Gate passed**  
   Issues  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [8 New 
issues](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=true)
   
   Measures  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3435=false=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [98.4% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_coverage=list)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0.0% Duplication on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_duplicated_lines_density=list)
  
 
   [See analysis details on 
SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3435)
   
   


-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-02-26 Thread via GitHub


mihaibudiu commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-1965637183

   Can anyone please review 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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-01-31 Thread via GitHub


mihaibudiu commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-1920187497

   This PR implements several instances of the EXTRACT function which weren't 
supported. I would appreciate a review.


-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2024-01-31 Thread via GitHub


sonarcloud[bot] commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-1920161628

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_calcite=3435) 
**Quality Gate passed**  
   The SonarCloud Quality Gate passed, but some issues were introduced.
   
   [10 New 
issues](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=true)
  
   [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3435=false=true)
  
   [98.4% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_coverage=list)
  
   [0.0% Duplication on New 
Code](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_duplicated_lines_density=list)
  
 
   [See analysis details on 
SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3435)
   
   


-- 
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-6015] AssertionError during optimization of EXTRACT expression [calcite]

2023-12-06 Thread via GitHub


sonarcloud[bot] commented on PR #3435:
URL: https://github.com/apache/calcite/pull/3435#issuecomment-1843654427

   Kudos, SonarCloud Quality Gate passed!  [![Quality Gate 
passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png
 'Quality Gate 
passed')](https://sonarcloud.io/dashboard?id=apache_calcite=3435)
   
   
[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png
 
'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=BUG)
 
[![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png
 
'A')](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=BUG)
 [0 
Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=BUG)
  
   
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png
 
'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=VULNERABILITY)
 
[![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png
 
'A')](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=VULNERABILITY)
 [0 
Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=VULNERABILITY)
  
   [![Security 
Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png
 'Security 
Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3435=false=SECURITY_HOTSPOT)
 
[![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png
 
'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3435=false=SECURITY_HOTSPOT)
 [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3435=false=SECURITY_HOTSPOT)
  
   [![Code 
Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png
 'Code 
Smell')](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=CODE_SMELL)
 
[![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png
 
'A')](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=CODE_SMELL)
 [10 Code 
Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3435=false=CODE_SMELL)
   
   
[![98.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png
 
'98.3%')](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_coverage=list)
 [98.3% 
Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_coverage=list)
  
   
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png
 
'0.0%')](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_duplicated_lines_density=list)
 [0.0% 
Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3435=new_duplicated_lines_density=list)
   
   


-- 
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