[GitHub] [calcite] sonarcloud[bot] commented on pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric
sonarcloud[bot] commented on PR #3031: URL: https://github.com/apache/calcite/pull/3031#issuecomment-1386252119 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3031) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3031&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3031&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3031&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3031&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3031&metric=new_coverage&view=list) [83.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3031&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3031&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3031&metric=new_duplicated_lines_density&view=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
[GitHub] [calcite] olivrlee commented on a diff in pull request #3023: [CALCITE-5180] Implement BigQuery Date/Time Type Aliases and Constructors
olivrlee commented on code in PR #3023:
URL: https://github.com/apache/calcite/pull/3023#discussion_r1072911432
##
babel/src/test/resources/sql/big-query.iq:
##
@@ -1097,12 +1097,74 @@ select unix_date(datetime '2008-12-25') as d;
#
# Return Data Type: DATE
-select date(2022, 11, 15) as d;
-++
-| d |
-++
-| 2022-11-15 |
-++
+select date(2022, 11, 15) as d1,
+ date(datetime "2008-01-01 01:03:05") as d2,
+ date(datetime(2008, 1, 1, 1, 3, 5)) as d3;
+++++
+| d1 | d2 | d3 |
+++++
+| 2022-11-15 | 2008-01-01 | 2008-01-01 |
+++++
+(1 row)
+
+!ok
+
+# Test timezone conversion when converting TIMESTAMP to DATE.
+# Denver observes DST whereas Phoenix does not.
+# Both cities have a -07:00 offset in winter, but Denver has -06:00 in summer.
+select date(timestamp("2008-06-21 06:30:00")) as sum_utc,
+ date(timestamp("2008-06-21 06:30:00"), "America/Denver") as sum_dst,
+ date(timestamp("2008-06-21 06:30:00"), "America/Phoenix") as sum_std,
+ date(timestamp("2008-12-21 06:30:00")) as win_utc,
+ date(timestamp("2008-12-21 06:30:00"), "America/Denver") as win_dst,
+ date(timestamp("2008-12-21 06:30:00"), "America/Phoenix") as win_std;
++++++++
+| sum_utc| sum_dst| sum_std| win_utc| win_dst| win_std|
++++++++
+| 2008-06-21 | 2008-06-21 | 2008-06-20 | 2008-12-21 | 2008-12-20 | 2008-12-20 |
++++++++
+(1 row)
+
+!ok
+
+#
+# DATETIME
Review Comment:
there's a set of DATETIME tests currently disabled around line 650 btw
https://github.com/apache/calcite/blob/6e5b2f22414b1c1617d0e721d6dcf93f1b82d673/babel/src/test/resources/sql/big-query.iq#L649
--
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
[GitHub] [calcite] birschick-bq commented on a diff in pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric
birschick-bq commented on code in PR #3031:
URL: https://github.com/apache/calcite/pull/3031#discussion_r1072879759
##
core/src/main/java/org/apache/calcite/rel/rules/ProjectAggregateMergeRule.java:
##
@@ -99,7 +100,8 @@ && kindCount(project.getProjects(), SqlKind.CASE) == 0) {
&& operands.get(1).getKind() == SqlKind.CAST
&& ((RexCall) operands.get(1)).operands.get(0).getKind()
== SqlKind.INPUT_REF
- && operands.get(2).getKind() == SqlKind.LITERAL) {
+ && operands.get(2).getKind() == SqlKind.LITERAL
+ && operands.get(2).getType().getFamily() ==
SqlTypeFamily.NUMERIC) {
Review Comment:
@clesaec
I agree with you on this. I believe there could be a conversion for
`BigDecimal` from `TIMESTAMP`.
--
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
[GitHub] [calcite] clesaec commented on a diff in pull request #3031: [CALCITE-5483] ProjectAggregateMergeRule throws exception if literal is non-numeric
clesaec commented on code in PR #3031:
URL: https://github.com/apache/calcite/pull/3031#discussion_r1072284872
##
core/src/main/java/org/apache/calcite/rel/rules/ProjectAggregateMergeRule.java:
##
@@ -99,7 +100,8 @@ && kindCount(project.getProjects(), SqlKind.CASE) == 0) {
&& operands.get(1).getKind() == SqlKind.CAST
&& ((RexCall) operands.get(1)).operands.get(0).getKind()
== SqlKind.INPUT_REF
- && operands.get(2).getKind() == SqlKind.LITERAL) {
+ && operands.get(2).getKind() == SqlKind.LITERAL
+ && operands.get(2).getType().getFamily() ==
SqlTypeFamily.NUMERIC) {
Review Comment:
I wonder where the code `literal.getValueAs(BigDecimal.class)` (line 111)
could work as [getValueAs
method](https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/rex/RexLiteral.java#L1029)
has no case for "clazz == BigDecimal" ... May be not related to this specific
case, but it seems there is also an issue on RexLiteral class.
--
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: [CALCITE-5475] Improve test coverage accuracy by aggregating modules
This is an automated email from the ASF dual-hosted git repository.
zabetak pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new fe821caaa5 [CALCITE-5475] Improve test coverage accuracy by
aggregating modules
fe821caaa5 is described below
commit fe821caaa5db0050bfa6f01a8c0397a20973138f
Author: Stamatis Zampetakis
AuthorDate: Fri Jan 13 17:02:53 2023 +0100
[CALCITE-5475] Improve test coverage accuracy by aggregating modules
Considering the modules in isolation leads to some modules
(e.g., testkit) reporting low coverage. The use of
jacoco-report-aggregation plugin alleviates the problem by aggregating
the coverage analysis from the individual modules into one unified
report.
While testing the changes there was one Jenkins job that got stuck
and kept running for almost ~24h. To prevent similar situation in the
future a 1h timeout is set in the code quality stage.
Close apache/calcite#3027
---
Jenkinsfile | 14 --
build.gradle.kts | 17 +
2 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/Jenkinsfile b/Jenkinsfile
index 2f2aee558f..53cc1e6f31 100644
--- a/Jenkinsfile
+++ b/Jenkinsfile
@@ -40,12 +40,14 @@ node('ubuntu') {
}
}
stage('Code Quality') {
-withEnv(["Path+JDK=$JAVA_JDK_17/bin","JAVA_HOME=$JAVA_JDK_17"]) {
- withCredentials([string(credentialsId: 'SONARCLOUD_TOKEN', variable:
'SONAR_TOKEN')]) {
-if ( env.BRANCH_NAME.startsWith("PR-") ) {
- sh './gradlew --no-parallel --no-daemon build jacocoTestReport sonar
-PenableJacoco -Dsonar.pullrequest.branch=${CHANGE_BRANCH}
-Dsonar.pullrequest.base=${CHANGE_TARGET} -Dsonar.pullrequest.key=${CHANGE_ID}
-Dsonar.login=${SONAR_TOKEN}'
-} else {
- sh './gradlew --no-parallel --no-daemon build jacocoTestReport sonar
-PenableJacoco -Dsonar.branch.name=${BRANCH_NAME} -Dsonar.login=${SONAR_TOKEN}'
+timeout(time: 1, unit: 'HOURS') {
+ withEnv(["Path+JDK=$JAVA_JDK_17/bin","JAVA_HOME=$JAVA_JDK_17"]) {
+withCredentials([string(credentialsId: 'SONARCLOUD_TOKEN', variable:
'SONAR_TOKEN')]) {
+ if ( env.BRANCH_NAME.startsWith("PR-") ) {
+sh './gradlew --no-parallel --no-daemon jacocoAggregateTestReport
sonar -PenableJacoco -Dsonar.pullrequest.branch=${CHANGE_BRANCH}
-Dsonar.pullrequest.base=${CHANGE_TARGET} -Dsonar.pullrequest.key=${CHANGE_ID}
-Dsonar.login=${SONAR_TOKEN}'
+ } else {
+sh './gradlew --no-parallel --no-daemon jacocoAggregateTestReport
sonar -PenableJacoco -Dsonar.branch.name=${BRANCH_NAME}
-Dsonar.login=${SONAR_TOKEN}'
+ }
}
}
}
diff --git a/build.gradle.kts b/build.gradle.kts
index ec1e8f9468..bbde9d140f 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -39,6 +39,7 @@ plugins {
checkstyle
calcite.buildext
jacoco
+id("jacoco-report-aggregation")
id("org.checkerframework") apply false
id("com.github.autostyle")
id("org.nosphere.apache.rat")
@@ -171,6 +172,16 @@ releaseParams {
}
}
+reporting {
+reports {
+if (enableJacoco) {
+val jacocoAggregateTestReport by
creating(JacocoCoverageReport::class) {
+testType.set(TestSuiteType.UNIT_TEST)
+}
+}
+}
+}
+
val javadocAggregate by tasks.registering(Javadoc::class) {
group = JavaBasePlugin.DOCUMENTATION_GROUP
description = "Generates aggregate javadoc for all the artifacts"
@@ -229,6 +240,11 @@ dependencies {
for (m in dataSetsForSqlline) {
sqllineClasspath(module(m))
}
+if (enableJacoco) {
+for (p in subprojects) {
+jacocoAggregation(p)
+}
+}
}
val buildSqllineClasspath by tasks.registering(Jar::class) {
@@ -300,6 +316,7 @@ fun
com.github.autostyle.gradle.BaseFormatExtension.license() {
sonarqube {
properties {
property("sonar.test.inclusions", "**/*Test*/**")
+property("sonar.coverage.jacoco.xmlReportPaths",
"$buildDir/reports/jacoco/jacocoAggregateTestReport/jacocoAggregateTestReport.xml")
}
}
[GitHub] [calcite] zabetak closed pull request #3027: [CALCITE-5475] Improve test coverage accuracy by aggregating modules
zabetak closed pull request #3027: [CALCITE-5475] Improve test coverage accuracy by aggregating modules URL: https://github.com/apache/calcite/pull/3027 -- 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
