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