DonnyZone commented on PR #3203:
URL: https://github.com/apache/calcite/pull/3203#issuecomment-1563744642
> > Sorry for the late reply. I make some tests in my local environment. The
optimization for `BinaryExpression` seems to be incorrect. The code after
optimization throws compilation
sonarcloud[bot] commented on PR #3214:
URL: https://github.com/apache/calcite/pull/3214#issuecomment-1563741869
Kudos, SonarCloud Quality Gate passed! [![Quality Gate
passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png
herunkang2018 commented on PR #3217:
URL: https://github.com/apache/calcite/pull/3217#issuecomment-1563681195
The improvement looks good to me, I have another question, is there other
case that can be pruned beside this PR's case?
--
This is an automated message from the Apache Git
sonarcloud[bot] commented on PR #3209:
URL: https://github.com/apache/calcite/pull/3209#issuecomment-1563581686
Kudos, SonarCloud Quality Gate passed! [![Quality Gate
passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png
mihaibudiu commented on PR #3145:
URL: https://github.com/apache/calcite/pull/3145#issuecomment-1563493613
Currently the "execute" method from Main only returns an integer and writes
all diagnostic to the supplied streams.
We have to make it return the actual TestStatistics object.
So
julianhyde commented on PR #3145:
URL: https://github.com/apache/calcite/pull/3145#issuecomment-1563491610
Rather than checking that the number of failures doesn't change, it's better
to check that there are no failures other than the list of 'expected failures'
You don't want the
zabetak commented on code in PR #3145:
URL: https://github.com/apache/calcite/pull/3145#discussion_r1205976037
##
plus/src/test/java/org/apache/calcite/slt/executors/CalciteExecutor.java:
##
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
zabetak commented on code in PR #3145:
URL: https://github.com/apache/calcite/pull/3145#discussion_r1205973836
##
plus/src/test/java/org/apache/calcite/slt/TestCalcite.java:
##
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ *
tanclary commented on code in PR #3214:
URL: https://github.com/apache/calcite/pull/3214#discussion_r1205820685
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -1897,6 +1930,29 @@ public static double atan2(double b0, double b1) {
return
julianhyde commented on code in PR #3209:
URL: https://github.com/apache/calcite/pull/3209#discussion_r1205725017
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -1688,6 +1692,27 @@ static class GroupingImplementor implements
AggImplementor
rubenada commented on code in PR #3209:
URL: https://github.com/apache/calcite/pull/3209#discussion_r1205653893
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -1688,6 +1692,27 @@ static class GroupingImplementor implements
AggImplementor {
rubenada commented on PR #3214:
URL: https://github.com/apache/calcite/pull/3214#issuecomment-1562918294
@tanclary I see you have participated on other trigonometric-related PRs,
would you like to take a look at this one?
I think it is in a good shape and I will proceed to merge (after
sonarcloud[bot] commented on PR #3225:
URL: https://github.com/apache/calcite/pull/3225#issuecomment-1562862638
Kudos, SonarCloud Quality Gate passed! [
--
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,
clayburn commented on PR #3223:
URL: https://github.com/apache/calcite/pull/3223#issuecomment-1562836473
> Overall LTGM. Is there a way to test that it works as expected before
merging?
Sure, a few things you can do:
* These are the two builds that have run so far for this PR in
clayburn commented on code in PR #3223:
URL: https://github.com/apache/calcite/pull/3223#discussion_r1205453958
##
settings.gradle.kts:
##
@@ -51,7 +53,8 @@ pluginManagement {
}
plugins {
-`gradle-enterprise`
+id("com.gradle.enterprise") version "3.13.2"
+
zstan commented on PR #3203:
URL: https://github.com/apache/calcite/pull/3203#issuecomment-1562446539
> Sorry for the late reply. I make some tests in my local environment. The
optimization for `BinaryExpression` seems to be incorrect. The code after
optimization throws compilation error.
zabetak commented on code in PR #3223:
URL: https://github.com/apache/calcite/pull/3223#discussion_r1205101753
##
settings.gradle.kts:
##
@@ -51,7 +53,8 @@ pluginManagement {
}
plugins {
-`gradle-enterprise`
+id("com.gradle.enterprise") version "3.13.2"
+
HanumathRao commented on code in PR #3193:
URL: https://github.com/apache/calcite/pull/3193#discussion_r1205057374
##
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##
@@ -6823,6 +6823,48 @@ private void checkSemiJoinRuleOnAntiJoin(RelOptRule
rule) {
HanumathRao commented on code in PR #3193:
URL: https://github.com/apache/calcite/pull/3193#discussion_r1205054838
##
core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java:
##
@@ -866,6 +866,8 @@ private static void matchFilter(SubQueryRemoveRule rule,
zoudan commented on code in PR #3224:
URL: https://github.com/apache/calcite/pull/3224#discussion_r1205054013
##
core/src/main/java/org/apache/calcite/util/RangeSets.java:
##
@@ -129,7 +129,7 @@ public static > int
hashCode(RangeSet rangeSet) {
public static > boolean
zoudan commented on code in PR #3224:
URL: https://github.com/apache/calcite/pull/3224#discussion_r1205051820
##
core/src/test/java/org/apache/calcite/util/RangeSetTest.java:
##
@@ -155,6 +155,9 @@ class RangeSetTest {
assertThat(RangeSets.isPoint(Range.atMost(0)),
sonarcloud[bot] commented on PR #3193:
URL: https://github.com/apache/calcite/pull/3193#issuecomment-1562349622
Kudos, SonarCloud Quality Gate passed! [