[GitHub] [calcite] sonarcloud[bot] commented on pull request #3405: [CALCITE-5961] Type inference of ARRAY_COMPACT is incorrect
sonarcloud[bot] commented on PR #3405: URL: https://github.com/apache/calcite/pull/3405#issuecomment-1702183325 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3405) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3405=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3405=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3405=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3405=new_coverage=list) [93.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3405=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3405=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3405=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
[GitHub] [calcite] chucheng92 commented on a diff in pull request #3405: [CALCITE-5961] Type inference of ARRAY_COMPACT is incorrect
chucheng92 commented on code in PR #3405:
URL: https://github.com/apache/calcite/pull/3405#discussion_r1312463704
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -5885,7 +5885,9 @@ private static void checkIf(SqlOperatorFixture f) {
final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
f.checkScalar("array_compact(array[null, 1, null, 2])", "[1, 2]",
-"INTEGER ARRAY NOT NULL");
+"INTEGER NOT NULL ARRAY NOT NULL");
+f.checkScalar("array_compact(array[1, 2])", "[1, 2]",
+"INTEGER NOT NULL ARRAY NOT NULL");
f.checkScalar("array_compact(array[null])", "[]",
"NULL ARRAY NOT NULL");
f.checkScalar("array_compact(array(null))", "[]",
Review Comment:
@NobiGo thank for reviewing~ Yes, it was introduced by the previous
commit/PR, I have removed it. PTAL.
--
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 #3391: [CALCITE-5933] Add SAFE_DIVIDE function (enabled in BigQuery library)
olivrlee commented on code in PR #3391:
URL: https://github.com/apache/calcite/pull/3391#discussion_r1312548850
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -7385,8 +7385,8 @@ private static void checkIf(SqlOperatorFixture f) {
@Test void testSafeAddFunc() {
final SqlOperatorFixture f0 =
fixture().setFor(SqlLibraryOperators.SAFE_ADD);
f0.checkFails("^safe_add(2, 3)^",
-"No match found for function signature "
-+ "SAFE_ADD\\(, \\)", false);
+ "No match found for function signature "
++ "SAFE_ADD\\(, \\)", false);
Review Comment:
is this indent still here or is it outdated?
--
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] sonarcloud[bot] commented on pull request #3399: [CALCITE-5962] Support parse Spark-style syntax LEFT ANTI JOIN in Babel parser
sonarcloud[bot] commented on PR #3399: URL: https://github.com/apache/calcite/pull/3399#issuecomment-1702136712 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3399) [](https://sonarcloud.io/project/issues?id=apache_calcite=3399=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3399=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3399=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3399=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3399=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3399=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3399=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3399=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3399=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3399=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3399=false=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3399=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3399=new_coverage=list) [84.6% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3399=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3399=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3399=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
[GitHub] [calcite] chucheng92 commented on a diff in pull request #3405: [CALCITE-5961] Type inference of ARRAY_COMPACT is incorrect
chucheng92 commented on code in PR #3405:
URL: https://github.com/apache/calcite/pull/3405#discussion_r1312463704
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -5885,7 +5885,9 @@ private static void checkIf(SqlOperatorFixture f) {
final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
f.checkScalar("array_compact(array[null, 1, null, 2])", "[1, 2]",
-"INTEGER ARRAY NOT NULL");
+"INTEGER NOT NULL ARRAY NOT NULL");
+f.checkScalar("array_compact(array[1, 2])", "[1, 2]",
+"INTEGER NOT NULL ARRAY NOT NULL");
f.checkScalar("array_compact(array[null])", "[]",
"NULL ARRAY NOT NULL");
f.checkScalar("array_compact(array(null))", "[]",
Review Comment:
@NobiGo thank for reviewing. Yes, it was introduced by the previous
commit/PR, should I can remove it in 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
[GitHub] [calcite] JiajunBernoulli commented on pull request #3382: [CALCITE-5944] RelMdRowCount can return more accurate rowCount for Sample
JiajunBernoulli commented on PR #3382: URL: https://github.com/apache/calcite/pull/3382#issuecomment-1702021370 @rubenada , would you please review 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
[GitHub] [calcite] JiajunBernoulli closed pull request #3283: [CALCITE-5805] SqlValidatorImpl throws AssertionError while validatin…
JiajunBernoulli closed pull request #3283: [CALCITE-5805] SqlValidatorImpl throws AssertionError while validatin… URL: https://github.com/apache/calcite/pull/3283 -- 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] chucheng92 commented on a diff in pull request #3405: [CALCITE-5961] Type inference of ARRAY_COMPACT is incorrect
chucheng92 commented on code in PR #3405:
URL: https://github.com/apache/calcite/pull/3405#discussion_r1312463704
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -5885,7 +5885,9 @@ private static void checkIf(SqlOperatorFixture f) {
final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
f.checkScalar("array_compact(array[null, 1, null, 2])", "[1, 2]",
-"INTEGER ARRAY NOT NULL");
+"INTEGER NOT NULL ARRAY NOT NULL");
+f.checkScalar("array_compact(array[1, 2])", "[1, 2]",
+"INTEGER NOT NULL ARRAY NOT NULL");
f.checkScalar("array_compact(array[null])", "[]",
"NULL ARRAY NOT NULL");
f.checkScalar("array_compact(array(null))", "[]",
Review Comment:
@NobiGo thank for reviewing. Yes, it was introduced by the previous
commit/PR, and I can remove it in this PR? WDYT?
--
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] NobiGo commented on a diff in pull request #3405: [CALCITE-5961] Type inference of ARRAY_COMPACT is incorrect
NobiGo commented on code in PR #3405:
URL: https://github.com/apache/calcite/pull/3405#discussion_r1312440471
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -5885,7 +5885,9 @@ private static void checkIf(SqlOperatorFixture f) {
final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
f.checkScalar("array_compact(array[null, 1, null, 2])", "[1, 2]",
-"INTEGER ARRAY NOT NULL");
+"INTEGER NOT NULL ARRAY NOT NULL");
+f.checkScalar("array_compact(array[1, 2])", "[1, 2]",
+"INTEGER NOT NULL ARRAY NOT NULL");
f.checkScalar("array_compact(array[null])", "[]",
"NULL ARRAY NOT NULL");
f.checkScalar("array_compact(array(null))", "[]",
Review Comment:
Two same tests for `array_compact(array[null])`?
--
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] NobiGo commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
NobiGo commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1312426914
##
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##
@@ -3773,6 +3774,35 @@ public void checkOrderBy(final boolean desc,
"empid=110; name=Theodore");
}
+ @Test void testExceptToDistinct() {
+final String sql = ""
++ "select \"empid\", \"name\" from \"hr\".\"emps\" where
\"deptno\"=10\n"
++ "except\n"
++ "select \"empid\", \"name\" from \"hr\".\"emps\" where
\"empid\">=150";
+CalciteAssert.hr()
+.query(sql)
+.withHook(Hook.PLANNER, (Consumer) planner ->
+planner.removeRule(ENUMERABLE_MINUS_RULE))
+.explainContains(""
++ "PLAN=EnumerableCalc(expr#0..3=[{inputs}], expr#4=[0],
expr#5=[>($t2, $t4)], "
++ "expr#6=[=($t3, $t4)], expr#7=[AND($t5, $t6)],
proj#0..1=[{exprs}], "
++ "$condition=[$t7])\n"
++ " EnumerableAggregate(group=[{0, 1}], agg#0=[COUNT() FILTER
$2], agg#1=[COUNT() "
++ "FILTER $3])\n"
++ "EnumerableCalc(expr#0..2=[{inputs}], expr#3=[0],
expr#4=[=($t2, $t3)], "
++ "expr#5=[1], expr#6=[=($t2, $t5)], proj#0..1=[{exprs}],
$f2=[$t4], $f3=[$t6])\n"
++ " EnumerableUnion(all=[true])\n"
++ "EnumerableCalc(expr#0..4=[{inputs}], expr#5=[0],
expr#6=[CAST($t1):INTEGER"
++ " NOT NULL], expr#7=[10], expr#8=[=($t6, $t7)], empid=[$t0],
name=[$t2], $f2=[$t5],"
++ " $condition=[$t8])\n"
++ " EnumerableTableScan(table=[[hr, emps]])\n"
++ "EnumerableCalc(expr#0..4=[{inputs}], expr#5=[1],
expr#6=[150], expr#7=[>="
++ "($t0, $t6)], empid=[$t0], name=[$t2], $f2=[$t5],
$condition=[$t7])\n"
++ " EnumerableTableScan(table=[[hr, emps]])\n")
+.returnsUnordered("empid=100; name=Bill",
Review Comment:
How about adding another test like:
```
with doubleEmp as (select empno, ename
from emp
union all
select empno, ename
from emp)
select empno, ename
from doubleEmp
except
select empno, ename
from emp;
```
##
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##
@@ -3773,6 +3774,35 @@ public void checkOrderBy(final boolean desc,
"empid=110; name=Theodore");
}
+ @Test void testExceptToDistinct() {
Review Comment:
I agree with @asolimando . Although the meanings of `MINUS` and `Except`
are the same, We still need to stay consistent.
##
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##
@@ -2699,6 +2699,35 @@ private void
checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
.check();
}
+ /** Tests {@link org.apache.calcite.rel.rules.MinusToDistinctRule},
Review Comment:
@asolimando Yes. Actually, The .iq file tries to make JdbcTest more simple
and convenient. But this PR needs to make the specific Rule work, the .iq file
didn't. In the other PR, we recommend removing tests to the .iq file. But this
PR is OK for now.
--
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-5911] Add REGEXP_EXTRACT_ALL function (enabled in BigQuery library)
This is an automated email from the ASF dual-hosted git repository.
tanner 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 ada4fe2715 [CALCITE-5911] Add REGEXP_EXTRACT_ALL function (enabled in
BigQuery library)
ada4fe2715 is described below
commit ada4fe2715aac8e30751cbcfb84e0b60d89db4ee
Author: Jerin John
AuthorDate: Tue Jul 25 15:29:31 2023 -0700
[CALCITE-5911] Add REGEXP_EXTRACT_ALL function (enabled in BigQuery library)
---
babel/src/test/resources/sql/big-query.iq | 59
.../calcite/adapter/enumerable/RexImpTable.java| 2 +
.../org/apache/calcite/runtime/SqlFunctions.java | 65 ++
.../calcite/sql/fun/SqlLibraryOperators.java | 9 +++
.../org/apache/calcite/util/BuiltInMethod.java | 2 +
.../org/apache/calcite/test/SqlFunctionsTest.java | 29 ++
site/_docs/reference.md| 1 +
.../org/apache/calcite/test/SqlOperatorTest.java | 23
8 files changed, 167 insertions(+), 23 deletions(-)
diff --git a/babel/src/test/resources/sql/big-query.iq
b/babel/src/test/resources/sql/big-query.iq
index 353b7c518c..f38032ae32 100755
--- a/babel/src/test/resources/sql/big-query.iq
+++ b/babel/src/test/resources/sql/big-query.iq
@@ -1114,6 +1114,65 @@ SELECT REGEXP_EXTRACT("abcadcabcaecghi", "a.c", 3, 0);
Invalid integer input '0' for argument 'occurrence' in REGEXP_EXTRACT
!error
+#
+# REGEXP_EXTRACT_ALL(value, regexp)
+#
+# Returns an array of all substrings in value that matches the regexp.
+# Returns an empty array if there is no match.
+# Returns an exception if regex is invalid or has more than one capturing
group.
+
+WITH code_markdown AS
+ (SELECT 'Try `function(x)` or `function(y)`' as code)
+SELECT
+ REGEXP_EXTRACT_ALL(code, '`(.+?)`') AS example
+FROM code_markdown;
+++
+| example|
+++
+| [function(x), function(y)] |
+++
+(1 row)
+
+!ok
+
+SELECT REGEXP_EXTRACT_ALL("abcadcabcaecghi", "abc(a.c)");
+++
+| EXPR$0 |
+++
+| [adc, aec] |
+++
+(1 row)
+
+!ok
+
+SELECT REGEXP_EXTRACT_ALL("abacadaeafa", "a.a");
++-+
+| EXPR$0 |
++-+
+| [aba, ada, afa] |
++-+
+(1 row)
+
+!ok
+
+SELECT REGEXP_EXTRACT_ALL("12345abc123", "a[0-9]");
+++
+| EXPR$0 |
+++
+| [] |
+++
+(1 row)
+
+!ok
+
+SELECT REGEXP_EXTRACT_ALL("abc def ghi", "{4,1}");
+Invalid regular expression for REGEXP_EXTRACT_ALL: 'Illegal repetition range
near index 4 {4,1} ^'
+!error
+
+SELECT REGEXP_EXTRACT_ALL("abcadcabcaecghi", "(a.c).(.*)$");
+Multiple capturing groups (count=2) not allowed in regex input for
REGEXP_EXTRACT_ALL
+!error
+
#
# REGEXP_SUBSTR(value, regexp[, position[, occurrence]])
#
diff --git
a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
index b9aac10a35..b4a5cb8ed9 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
@@ -220,6 +220,7 @@ import static
org.apache.calcite.sql.fun.SqlLibraryOperators.PARSE_URL;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.POW;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_CONTAINS;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_EXTRACT;
+import static
org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_EXTRACT_ALL;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_REPLACE;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REPEAT;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REVERSE;
@@ -573,6 +574,7 @@ public class RexImpTable {
defineReflective(REGEXP_CONTAINS, BuiltInMethod.REGEXP_CONTAINS.method);
defineReflective(REGEXP_EXTRACT, BuiltInMethod.REGEXP_EXTRACT2.method,
BuiltInMethod.REGEXP_EXTRACT3.method,
BuiltInMethod.REGEXP_EXTRACT4.method);
+ defineReflective(REGEXP_EXTRACT_ALL,
BuiltInMethod.REGEXP_EXTRACT_ALL.method);
map.put(TRIM, new TrimImplementor());
diff --git a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
index 0afe39748c..c64b70ad18 100644
--- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -388,22 +388,34 @@ public class SqlFunctions {
.maximumSize(FUNCTION_LEVEL_CACHE_MAX_SIZE.value())
[GitHub] [calcite] tanclary merged pull request #3387: [CALCITE-5911] Add REGEXP_EXTRACT_ALL function (enabled in BigQuery library)
tanclary merged PR #3387: URL: https://github.com/apache/calcite/pull/3387 -- 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] sonarcloud[bot] commented on pull request #3387: [CALCITE-5911] Add REGEXP_EXTRACT_ALL function (enabled in BigQuery library)
sonarcloud[bot] commented on PR #3387: URL: https://github.com/apache/calcite/pull/3387#issuecomment-1701561915 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3387) [](https://sonarcloud.io/project/issues?id=apache_calcite=3387=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3387=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3387=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3387=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3387=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3387=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3387=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3387=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3387=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3387=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3387=false=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3387=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3387=new_coverage=list) [95.7% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3387=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3387=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3387=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
[GitHub] [calcite] Anthrino commented on a diff in pull request #3387: [CALCITE-5911] Add REGEXP_EXTRACT_ALL function (enabled in BigQuery library)
Anthrino commented on code in PR #3387:
URL: https://github.com/apache/calcite/pull/3387#discussion_r1312041006
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -4675,6 +4675,29 @@ private static void checkIf(SqlOperatorFixture f) {
f.checkQuery("select regexp_extract('a9cadca5c4aecghi', 'a[0-9]c', 1, 3)");
}
+ @Test void testRegexpExtractAllFunc() {
+final SqlOperatorFixture f =
+
fixture().setFor(SqlLibraryOperators.REGEXP_EXTRACT_ALL).withLibrary(SqlLibrary.BIG_QUERY);
+
+f.checkScalar("regexp_extract_all('a9cadca5c4aecghi', 'a[0-9]c')", "[a9c,
a5c]", "CHAR(16) "
++ "NOT NULL ARRAY NOT NULL");
+f.checkScalar("regexp_extract_all('abcde', '.')", "[a, b, c, d, e]",
"CHAR(5) NOT NULL ARRAY "
++ "NOT NULL");
+f.checkScalar("regexp_extract_all('abcadcabcaecghi', 'ac')", "[]",
"CHAR(15) NOT NULL ARRAY "
++ "NOT NULL");
+f.checkScalar("regexp_extract_all('f...@bar.com, f...@gmail.com,
f...@outlook.com', "
++ "'@[a-zA-Z0-9-]+\\.[a-zA-Z0-9-.]+')", "[@bar.com, @gmail.com,
@outlook.com]", "CHAR(43) "
++ "NOT NULL ARRAY NOT NULL");
+
+f.checkNull("regexp_extract_all('abc def ghi', cast(null as varchar))");
Review Comment:
Good catch @ILuffZhe, thanks!
Made the change in sql lib operator definition, I missed that assuming its
always returning an array (empty for no matches).
--
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] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311961669
##
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##
@@ -2699,6 +2699,33 @@ private void
checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
.check();
}
+ /** Tests {@link org.apache.calcite.rel.rules.MinusToDistinctRule},
+ * which rewrites an {@link Minus} operator with 3 inputs. */
+ @Test void testMinusToDistinct() {
+final String sql = "select EMPNO,ENAME,JOB from emp where deptno = 10\n"
++ "except\n"
++ "select EMPNO,ENAME,JOB from emp where deptno = 20\n"
++ "except\n"
++ "select EMPNO,ENAME,JOB from emp where deptno = 30\n";
+sql(sql)
+.withRule(CoreRules.MINUS_MERGE, CoreRules.MINUS_TO_DISTINCT,
CoreRules.PROJECT_MERGE)
+.check();
+ }
+
+ /** Tests {@link org.apache.calcite.rel.rules.MinusToDistinctRule},
+ * correctly ignores an {@code EXCEPT ALL}. It can only handle
+ * {@code EXCEPT DISTINCT}. */
+ @Test void testMinusToDistinctAll() {
Review Comment:
This test covers `A except B except all C`, can you also add a test covering
`A except all B except C`?
--
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] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311953152
##
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##
@@ -2699,6 +2699,33 @@ private void
checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
.check();
}
+ /** Tests {@link org.apache.calcite.rel.rules.MinusToDistinctRule},
+ * which rewrites an {@link Minus} operator with 3 inputs. */
+ @Test void testMinusToDistinct() {
+final String sql = "select EMPNO,ENAME,JOB from emp where deptno = 10\n"
Review Comment:
Nit: can we add a space after commas like in all the other tests? Like
`EMPNO, ENAME` etc.?
--
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] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311953152
##
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##
@@ -2699,6 +2699,33 @@ private void
checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
.check();
}
+ /** Tests {@link org.apache.calcite.rel.rules.MinusToDistinctRule},
+ * which rewrites an {@link Minus} operator with 3 inputs. */
+ @Test void testMinusToDistinct() {
+final String sql = "select EMPNO,ENAME,JOB from emp where deptno = 10\n"
Review Comment:
Nit: can we add a space after commas? Like `EMPNO, ENAME` etc.?
--
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] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311949697
##
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##
@@ -2699,6 +2699,35 @@ private void
checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
.check();
}
+ /** Tests {@link org.apache.calcite.rel.rules.MinusToDistinctRule},
Review Comment:
We are abusing `JdbcTest` but there seems to be no better way at the moment
(https://lists.apache.org/thread/r4yhn77m92fodmz8xm6k40sfd130w7hh), and it's
definitely better to have a test here than no test at all.
--
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] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311942668
##
core/src/main/java/org/apache/calcite/rel/rules/MinusToDistinctRule.java:
##
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.logical.LogicalMinus;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import org.immutables.value.Value;
+
+import java.math.BigDecimal;
+
+/**
+ * Planner rule that translates a distinct
+ * {@link org.apache.calcite.rel.core.Minus}
+ * (all = false)
+ * into a group of operators composed of
+ * {@link org.apache.calcite.rel.core.Union},
+ * {@link org.apache.calcite.rel.core.Aggregate},
+ * {@link org.apache.calcite.rel.core.Filter},etc.
+ *
+ * For example, the query plan
+
+ * {@code
+ * LogicalMinus(all=[false])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * will convert to
+ *
+ * {@code
+ * LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ *LogicalFilter(condition=[AND(>($3, 0), =($4, 0))])
+ * LogicalAggregate(group=[{0, 1, 2}], agg#0=[COUNT() FILTER $3],
agg#1=[COUNT() FILTER $4])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[=($3, 0)],
$f4=[=($3, 1)])
+ * LogicalUnion(all=[true])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[0])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[1])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * @see CoreRules#MINUS_TO_DISTINCT
+ */
+@Value.Enclosing
+public class MinusToDistinctRule
+extends RelRule
+implements TransformationRule {
+
+ protected MinusToDistinctRule(Config config) {
+super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public MinusToDistinctRule(Class minusClass,
+ RelBuilderFactory relBuilderFactory) {
+
this(MinusToDistinctRule.Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+.as(MinusToDistinctRule.Config.class)
+.withOperandFor(minusClass));
+ }
+
+ @Override public void onMatch(RelOptRuleCall call) {
+Minus minus = call.rel(0);
+
+if (minus.all) {
+ // Nothing we can do
+ return;
+}
+
+final RelOptCluster cluster = minus.getCluster();
+final RelBuilder relBuilder = call.builder();
+final RexBuilder rexBuilder = cluster.getRexBuilder();
+final int branchCount = minus.getInputs().size();
+
+// For each child branch in minus,add a column which indicates branch index
+//
+// e.g. select EMPNO from emp -> select EMPNO,0 from emp
+// 0 indicates that it comes from minus the first child branch
+for (int i = 0; i < branchCount; i++) {
+ relBuilder.push(minus.getInput(i));
+ relBuilder.projectPlus(relBuilder.literal(new BigDecimal(i)));
+}
+
+// create a union above all the branches
+relBuilder.union(true, branchCount);
+
+final RelNode union = relBuilder.peek();
+final int originalFieldCnt = union.getRowType().getFieldCount() - 1;
+
+ImmutableList.Builder projects = ImmutableList.builder();
+// skip the branch index column
+
[GitHub] [calcite] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311937062
##
core/src/main/java/org/apache/calcite/rel/rules/MinusToDistinctRule.java:
##
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.logical.LogicalMinus;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import org.immutables.value.Value;
+
+import java.math.BigDecimal;
+
+/**
+ * Planner rule that translates a distinct
+ * {@link org.apache.calcite.rel.core.Minus}
+ * (all = false)
+ * into a group of operators composed of
+ * {@link org.apache.calcite.rel.core.Union},
+ * {@link org.apache.calcite.rel.core.Aggregate},
+ * {@link org.apache.calcite.rel.core.Filter},etc.
+ *
+ * For example, the query plan
+
+ * {@code
+ * LogicalMinus(all=[false])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * will convert to
+ *
+ * {@code
+ * LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ *LogicalFilter(condition=[AND(>($3, 0), =($4, 0))])
+ * LogicalAggregate(group=[{0, 1, 2}], agg#0=[COUNT() FILTER $3],
agg#1=[COUNT() FILTER $4])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[=($3, 0)],
$f4=[=($3, 1)])
+ * LogicalUnion(all=[true])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[0])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[1])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * @see CoreRules#MINUS_TO_DISTINCT
+ */
+@Value.Enclosing
+public class MinusToDistinctRule
+extends RelRule
+implements TransformationRule {
+
+ protected MinusToDistinctRule(Config config) {
+super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public MinusToDistinctRule(Class minusClass,
+ RelBuilderFactory relBuilderFactory) {
+
this(MinusToDistinctRule.Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+.as(MinusToDistinctRule.Config.class)
+.withOperandFor(minusClass));
+ }
+
+ @Override public void onMatch(RelOptRuleCall call) {
+Minus minus = call.rel(0);
+
+if (minus.all) {
+ // Nothing we can do
+ return;
+}
+
+final RelOptCluster cluster = minus.getCluster();
+final RelBuilder relBuilder = call.builder();
+final RexBuilder rexBuilder = cluster.getRexBuilder();
+final int branchCount = minus.getInputs().size();
+
+// For each child branch in minus,add a column which indicates branch index
+//
+// e.g. select EMPNO from emp -> select EMPNO,0 from emp
+// 0 indicates that it comes from minus the first child branch
+for (int i = 0; i < branchCount; i++) {
+ relBuilder.push(minus.getInput(i));
+ relBuilder.projectPlus(relBuilder.literal(new BigDecimal(i)));
+}
+
+// create a union above all the branches
+relBuilder.union(true, branchCount);
+
+final RelNode union = relBuilder.peek();
+final int originalFieldCnt = union.getRowType().getFieldCount() - 1;
+
+ImmutableList.Builder projects = ImmutableList.builder();
+// skip the branch index column
+
[GitHub] [calcite] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311934566
##
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##
@@ -3773,6 +3774,35 @@ public void checkOrderBy(final boolean desc,
"empid=110; name=Theodore");
}
+ @Test void testExceptToDistinct() {
+final String sql = ""
++ "select \"empid\", \"name\" from \"hr\".\"emps\" where
\"deptno\"=10\n"
++ "except\n"
++ "select \"empid\", \"name\" from \"hr\".\"emps\" where
\"empid\">=150";
+CalciteAssert.hr()
+.query(sql)
+.withHook(Hook.PLANNER, (Consumer) planner ->
+planner.removeRule(ENUMERABLE_MINUS_RULE))
Review Comment:
Can you elaborate on why you need to exclude this rule in the test?
--
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] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311933809
##
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##
@@ -3773,6 +3774,35 @@ public void checkOrderBy(final boolean desc,
"empid=110; name=Theodore");
}
+ @Test void testExceptToDistinct() {
Review Comment:
Isn't this test covering `MinusToDistinctRule`? If so, the test name should
be `testMinusToDistinct` or similar. Please check.
--
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] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311931518
##
core/src/main/java/org/apache/calcite/rel/rules/MinusToDistinctRule.java:
##
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.logical.LogicalMinus;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import org.immutables.value.Value;
+
+import java.math.BigDecimal;
+
+/**
+ * Planner rule that translates a distinct
+ * {@link org.apache.calcite.rel.core.Minus}
+ * (all = false)
+ * into a group of operators composed of
+ * {@link org.apache.calcite.rel.core.Union},
+ * {@link org.apache.calcite.rel.core.Aggregate},
+ * {@link org.apache.calcite.rel.core.Filter},etc.
+ *
+ * For example, the query plan
+
+ * {@code
+ * LogicalMinus(all=[false])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * will convert to
+ *
+ * {@code
+ * LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ *LogicalFilter(condition=[AND(>($3, 0), =($4, 0))])
+ * LogicalAggregate(group=[{0, 1, 2}], agg#0=[COUNT() FILTER $3],
agg#1=[COUNT() FILTER $4])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[=($3, 0)],
$f4=[=($3, 1)])
+ * LogicalUnion(all=[true])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[0])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[1])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * @see CoreRules#MINUS_TO_DISTINCT
+ */
+@Value.Enclosing
+public class MinusToDistinctRule
+extends RelRule
+implements TransformationRule {
+
+ protected MinusToDistinctRule(Config config) {
+super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public MinusToDistinctRule(Class minusClass,
+ RelBuilderFactory relBuilderFactory) {
+
this(MinusToDistinctRule.Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+.as(MinusToDistinctRule.Config.class)
+.withOperandFor(minusClass));
+ }
+
+ @Override public void onMatch(RelOptRuleCall call) {
+Minus minus = call.rel(0);
+
+if (minus.all) {
+ // Nothing we can do
+ return;
+}
+
+final RelOptCluster cluster = minus.getCluster();
+final RelBuilder relBuilder = call.builder();
+final RexBuilder rexBuilder = cluster.getRexBuilder();
+final int branchCount = minus.getInputs().size();
+
+// For each child branch in minus,add a column which indicates branch index
+//
+// e.g. select EMPNO from emp -> select EMPNO,0 from emp
+// 0 indicates that it comes from minus the first child branch
+for (int i = 0; i < branchCount; i++) {
+ relBuilder.push(minus.getInput(i));
+ relBuilder.projectPlus(relBuilder.literal(new BigDecimal(i)));
+}
+
+// create a union above all the branches
+relBuilder.union(true, branchCount);
+
+final RelNode union = relBuilder.peek();
+final int originalFieldCnt = union.getRowType().getFieldCount() - 1;
+
+ImmutableList.Builder projects = ImmutableList.builder();
+// skip the branch index column
+
[GitHub] [calcite] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311890061
##
core/src/main/java/org/apache/calcite/rel/rules/MinusToDistinctRule.java:
##
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.logical.LogicalMinus;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import org.immutables.value.Value;
+
+import java.math.BigDecimal;
+
+/**
+ * Planner rule that translates a distinct
+ * {@link org.apache.calcite.rel.core.Minus}
+ * (all = false)
+ * into a group of operators composed of
+ * {@link org.apache.calcite.rel.core.Union},
+ * {@link org.apache.calcite.rel.core.Aggregate},
+ * {@link org.apache.calcite.rel.core.Filter},etc.
+ *
+ * For example, the query plan
+
+ * {@code
+ * LogicalMinus(all=[false])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * will convert to
+ *
+ * {@code
+ * LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ *LogicalFilter(condition=[AND(>($3, 0), =($4, 0))])
+ * LogicalAggregate(group=[{0, 1, 2}], agg#0=[COUNT() FILTER $3],
agg#1=[COUNT() FILTER $4])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[=($3, 0)],
$f4=[=($3, 1)])
+ * LogicalUnion(all=[true])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[0])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[1])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * @see CoreRules#MINUS_TO_DISTINCT
+ */
+@Value.Enclosing
+public class MinusToDistinctRule
+extends RelRule
+implements TransformationRule {
+
+ protected MinusToDistinctRule(Config config) {
+super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public MinusToDistinctRule(Class minusClass,
+ RelBuilderFactory relBuilderFactory) {
+
this(MinusToDistinctRule.Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+.as(MinusToDistinctRule.Config.class)
+.withOperandFor(minusClass));
+ }
+
+ @Override public void onMatch(RelOptRuleCall call) {
+Minus minus = call.rel(0);
+
+if (minus.all) {
+ // Nothing we can do
+ return;
+}
+
+final RelOptCluster cluster = minus.getCluster();
+final RelBuilder relBuilder = call.builder();
+final RexBuilder rexBuilder = cluster.getRexBuilder();
+final int branchCount = minus.getInputs().size();
+
+// For each child branch in minus,add a column which indicates branch index
Review Comment:
Nit:
```suggestion
// For each child branch in minus, add a column which indicates the
branch index
```
--
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] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311895577
##
core/src/main/java/org/apache/calcite/rel/rules/MinusToDistinctRule.java:
##
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.logical.LogicalMinus;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import org.immutables.value.Value;
+
+import java.math.BigDecimal;
+
+/**
+ * Planner rule that translates a distinct
+ * {@link org.apache.calcite.rel.core.Minus}
+ * (all = false)
+ * into a group of operators composed of
+ * {@link org.apache.calcite.rel.core.Union},
+ * {@link org.apache.calcite.rel.core.Aggregate},
+ * {@link org.apache.calcite.rel.core.Filter},etc.
+ *
+ * For example, the query plan
+
+ * {@code
+ * LogicalMinus(all=[false])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * will convert to
+ *
+ * {@code
+ * LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ *LogicalFilter(condition=[AND(>($3, 0), =($4, 0))])
+ * LogicalAggregate(group=[{0, 1, 2}], agg#0=[COUNT() FILTER $3],
agg#1=[COUNT() FILTER $4])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[=($3, 0)],
$f4=[=($3, 1)])
+ * LogicalUnion(all=[true])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[0])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[1])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * @see CoreRules#MINUS_TO_DISTINCT
+ */
+@Value.Enclosing
+public class MinusToDistinctRule
+extends RelRule
+implements TransformationRule {
+
+ protected MinusToDistinctRule(Config config) {
+super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public MinusToDistinctRule(Class minusClass,
+ RelBuilderFactory relBuilderFactory) {
+
this(MinusToDistinctRule.Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+.as(MinusToDistinctRule.Config.class)
+.withOperandFor(minusClass));
+ }
+
+ @Override public void onMatch(RelOptRuleCall call) {
+Minus minus = call.rel(0);
+
+if (minus.all) {
+ // Nothing we can do
+ return;
+}
+
+final RelOptCluster cluster = minus.getCluster();
+final RelBuilder relBuilder = call.builder();
+final RexBuilder rexBuilder = cluster.getRexBuilder();
+final int branchCount = minus.getInputs().size();
+
+// For each child branch in minus,add a column which indicates branch index
+//
+// e.g. select EMPNO from emp -> select EMPNO,0 from emp
+// 0 indicates that it comes from minus the first child branch
Review Comment:
```suggestion
// 0 indicates that it comes from the first branch (operand) of the
minus operator
```
--
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] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311891801
##
core/src/main/java/org/apache/calcite/rel/rules/MinusToDistinctRule.java:
##
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.logical.LogicalMinus;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import org.immutables.value.Value;
+
+import java.math.BigDecimal;
+
+/**
+ * Planner rule that translates a distinct
+ * {@link org.apache.calcite.rel.core.Minus}
+ * (all = false)
+ * into a group of operators composed of
+ * {@link org.apache.calcite.rel.core.Union},
+ * {@link org.apache.calcite.rel.core.Aggregate},
+ * {@link org.apache.calcite.rel.core.Filter},etc.
+ *
+ * For example, the query plan
+
+ * {@code
+ * LogicalMinus(all=[false])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * will convert to
+ *
+ * {@code
+ * LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ *LogicalFilter(condition=[AND(>($3, 0), =($4, 0))])
+ * LogicalAggregate(group=[{0, 1, 2}], agg#0=[COUNT() FILTER $3],
agg#1=[COUNT() FILTER $4])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[=($3, 0)],
$f4=[=($3, 1)])
+ * LogicalUnion(all=[true])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[0])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[1])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * @see CoreRules#MINUS_TO_DISTINCT
+ */
+@Value.Enclosing
+public class MinusToDistinctRule
+extends RelRule
+implements TransformationRule {
+
+ protected MinusToDistinctRule(Config config) {
+super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public MinusToDistinctRule(Class minusClass,
+ RelBuilderFactory relBuilderFactory) {
+
this(MinusToDistinctRule.Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+.as(MinusToDistinctRule.Config.class)
+.withOperandFor(minusClass));
+ }
+
+ @Override public void onMatch(RelOptRuleCall call) {
+Minus minus = call.rel(0);
+
+if (minus.all) {
+ // Nothing we can do
+ return;
+}
+
+final RelOptCluster cluster = minus.getCluster();
+final RelBuilder relBuilder = call.builder();
+final RexBuilder rexBuilder = cluster.getRexBuilder();
+final int branchCount = minus.getInputs().size();
+
+// For each child branch in minus,add a column which indicates branch index
+//
+// e.g. select EMPNO from emp -> select EMPNO,0 from emp
+// 0 indicates that it comes from minus the first child branch
+for (int i = 0; i < branchCount; i++) {
+ relBuilder.push(minus.getInput(i));
+ relBuilder.projectPlus(relBuilder.literal(new BigDecimal(i)));
+}
+
+// create a union above all the branches
+relBuilder.union(true, branchCount);
+
+final RelNode union = relBuilder.peek();
+final int originalFieldCnt = union.getRowType().getFieldCount() - 1;
+
+ImmutableList.Builder projects = ImmutableList.builder();
+// skip the branch index column
+
[GitHub] [calcite] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311891801
##
core/src/main/java/org/apache/calcite/rel/rules/MinusToDistinctRule.java:
##
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.logical.LogicalMinus;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import org.immutables.value.Value;
+
+import java.math.BigDecimal;
+
+/**
+ * Planner rule that translates a distinct
+ * {@link org.apache.calcite.rel.core.Minus}
+ * (all = false)
+ * into a group of operators composed of
+ * {@link org.apache.calcite.rel.core.Union},
+ * {@link org.apache.calcite.rel.core.Aggregate},
+ * {@link org.apache.calcite.rel.core.Filter},etc.
+ *
+ * For example, the query plan
+
+ * {@code
+ * LogicalMinus(all=[false])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * will convert to
+ *
+ * {@code
+ * LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ *LogicalFilter(condition=[AND(>($3, 0), =($4, 0))])
+ * LogicalAggregate(group=[{0, 1, 2}], agg#0=[COUNT() FILTER $3],
agg#1=[COUNT() FILTER $4])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[=($3, 0)],
$f4=[=($3, 1)])
+ * LogicalUnion(all=[true])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[0])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[1])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * @see CoreRules#MINUS_TO_DISTINCT
+ */
+@Value.Enclosing
+public class MinusToDistinctRule
+extends RelRule
+implements TransformationRule {
+
+ protected MinusToDistinctRule(Config config) {
+super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public MinusToDistinctRule(Class minusClass,
+ RelBuilderFactory relBuilderFactory) {
+
this(MinusToDistinctRule.Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+.as(MinusToDistinctRule.Config.class)
+.withOperandFor(minusClass));
+ }
+
+ @Override public void onMatch(RelOptRuleCall call) {
+Minus minus = call.rel(0);
+
+if (minus.all) {
+ // Nothing we can do
+ return;
+}
+
+final RelOptCluster cluster = minus.getCluster();
+final RelBuilder relBuilder = call.builder();
+final RexBuilder rexBuilder = cluster.getRexBuilder();
+final int branchCount = minus.getInputs().size();
+
+// For each child branch in minus,add a column which indicates branch index
+//
+// e.g. select EMPNO from emp -> select EMPNO,0 from emp
+// 0 indicates that it comes from minus the first child branch
+for (int i = 0; i < branchCount; i++) {
+ relBuilder.push(minus.getInput(i));
+ relBuilder.projectPlus(relBuilder.literal(new BigDecimal(i)));
+}
+
+// create a union above all the branches
+relBuilder.union(true, branchCount);
+
+final RelNode union = relBuilder.peek();
+final int originalFieldCnt = union.getRowType().getFieldCount() - 1;
+
+ImmutableList.Builder projects = ImmutableList.builder();
+// skip the branch index column
+
[GitHub] [calcite] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311890731
##
core/src/main/java/org/apache/calcite/rel/rules/MinusToDistinctRule.java:
##
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.logical.LogicalMinus;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import org.immutables.value.Value;
+
+import java.math.BigDecimal;
+
+/**
+ * Planner rule that translates a distinct
+ * {@link org.apache.calcite.rel.core.Minus}
+ * (all = false)
+ * into a group of operators composed of
+ * {@link org.apache.calcite.rel.core.Union},
+ * {@link org.apache.calcite.rel.core.Aggregate},
+ * {@link org.apache.calcite.rel.core.Filter},etc.
+ *
+ * For example, the query plan
+
+ * {@code
+ * LogicalMinus(all=[false])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * will convert to
+ *
+ * {@code
+ * LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ *LogicalFilter(condition=[AND(>($3, 0), =($4, 0))])
+ * LogicalAggregate(group=[{0, 1, 2}], agg#0=[COUNT() FILTER $3],
agg#1=[COUNT() FILTER $4])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[=($3, 0)],
$f4=[=($3, 1)])
+ * LogicalUnion(all=[true])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[0])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[1])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * @see CoreRules#MINUS_TO_DISTINCT
+ */
+@Value.Enclosing
+public class MinusToDistinctRule
+extends RelRule
+implements TransformationRule {
+
+ protected MinusToDistinctRule(Config config) {
+super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public MinusToDistinctRule(Class minusClass,
+ RelBuilderFactory relBuilderFactory) {
+
this(MinusToDistinctRule.Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+.as(MinusToDistinctRule.Config.class)
+.withOperandFor(minusClass));
+ }
+
+ @Override public void onMatch(RelOptRuleCall call) {
+Minus minus = call.rel(0);
+
+if (minus.all) {
+ // Nothing we can do
+ return;
+}
+
+final RelOptCluster cluster = minus.getCluster();
+final RelBuilder relBuilder = call.builder();
+final RexBuilder rexBuilder = cluster.getRexBuilder();
+final int branchCount = minus.getInputs().size();
+
+// For each child branch in minus,add a column which indicates branch index
+//
+// e.g. select EMPNO from emp -> select EMPNO,0 from emp
Review Comment:
Nit: missing comma and space after comma
```suggestion
// e.g., select EMPNO from emp -> select EMPNO, 0 from emp
```
--
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] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311890061
##
core/src/main/java/org/apache/calcite/rel/rules/MinusToDistinctRule.java:
##
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.logical.LogicalMinus;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import org.immutables.value.Value;
+
+import java.math.BigDecimal;
+
+/**
+ * Planner rule that translates a distinct
+ * {@link org.apache.calcite.rel.core.Minus}
+ * (all = false)
+ * into a group of operators composed of
+ * {@link org.apache.calcite.rel.core.Union},
+ * {@link org.apache.calcite.rel.core.Aggregate},
+ * {@link org.apache.calcite.rel.core.Filter},etc.
+ *
+ * For example, the query plan
+
+ * {@code
+ * LogicalMinus(all=[false])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * will convert to
+ *
+ * {@code
+ * LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ *LogicalFilter(condition=[AND(>($3, 0), =($4, 0))])
+ * LogicalAggregate(group=[{0, 1, 2}], agg#0=[COUNT() FILTER $3],
agg#1=[COUNT() FILTER $4])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[=($3, 0)],
$f4=[=($3, 1)])
+ * LogicalUnion(all=[true])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[0])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[1])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * @see CoreRules#MINUS_TO_DISTINCT
+ */
+@Value.Enclosing
+public class MinusToDistinctRule
+extends RelRule
+implements TransformationRule {
+
+ protected MinusToDistinctRule(Config config) {
+super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public MinusToDistinctRule(Class minusClass,
+ RelBuilderFactory relBuilderFactory) {
+
this(MinusToDistinctRule.Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+.as(MinusToDistinctRule.Config.class)
+.withOperandFor(minusClass));
+ }
+
+ @Override public void onMatch(RelOptRuleCall call) {
+Minus minus = call.rel(0);
+
+if (minus.all) {
+ // Nothing we can do
+ return;
+}
+
+final RelOptCluster cluster = minus.getCluster();
+final RelBuilder relBuilder = call.builder();
+final RexBuilder rexBuilder = cluster.getRexBuilder();
+final int branchCount = minus.getInputs().size();
+
+// For each child branch in minus,add a column which indicates branch index
Review Comment:
Nit: missing space after comma
```suggestion
// For each child branch in minus, add a column which indicates branch
index
```
--
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] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311885728
##
core/src/main/java/org/apache/calcite/rel/rules/MinusToDistinctRule.java:
##
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.logical.LogicalMinus;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import org.immutables.value.Value;
+
+import java.math.BigDecimal;
+
+/**
+ * Planner rule that translates a distinct
+ * {@link org.apache.calcite.rel.core.Minus}
+ * (all = false)
+ * into a group of operators composed of
+ * {@link org.apache.calcite.rel.core.Union},
+ * {@link org.apache.calcite.rel.core.Aggregate},
+ * {@link org.apache.calcite.rel.core.Filter},etc.
+ *
+ * For example, the query plan
+
+ * {@code
+ * LogicalMinus(all=[false])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * will convert to
+ *
+ * {@code
+ * LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ *LogicalFilter(condition=[AND(>($3, 0), =($4, 0))])
+ * LogicalAggregate(group=[{0, 1, 2}], agg#0=[COUNT() FILTER $3],
agg#1=[COUNT() FILTER $4])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[=($3, 0)],
$f4=[=($3, 1)])
+ * LogicalUnion(all=[true])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[0])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[1])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * @see CoreRules#MINUS_TO_DISTINCT
+ */
+@Value.Enclosing
+public class MinusToDistinctRule
+extends RelRule
+implements TransformationRule {
+
+ protected MinusToDistinctRule(Config config) {
+super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public MinusToDistinctRule(Class minusClass,
+ RelBuilderFactory relBuilderFactory) {
+
this(MinusToDistinctRule.Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+.as(MinusToDistinctRule.Config.class)
+.withOperandFor(minusClass));
+ }
+
+ @Override public void onMatch(RelOptRuleCall call) {
+Minus minus = call.rel(0);
Review Comment:
```suggestion
final Minus minus = call.rel(0);
```
Since you have marked all other variables as such, it's sending the message
that it will mutate if you don't mark it final too.
--
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] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311847209
##
core/src/main/java/org/apache/calcite/rel/rules/MinusToDistinctRule.java:
##
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.logical.LogicalMinus;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import org.immutables.value.Value;
+
+import java.math.BigDecimal;
+
+/**
+ * Planner rule that translates a distinct
+ * {@link org.apache.calcite.rel.core.Minus}
+ * (all = false)
+ * into a group of operators composed of
+ * {@link org.apache.calcite.rel.core.Union},
+ * {@link org.apache.calcite.rel.core.Aggregate},
+ * {@link org.apache.calcite.rel.core.Filter},etc.
+ *
+ * For example, the query plan
+
+ * {@code
+ * LogicalMinus(all=[false])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * will convert to
+ *
+ * {@code
+ * LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ *LogicalFilter(condition=[AND(>($3, 0), =($4, 0))])
+ * LogicalAggregate(group=[{0, 1, 2}], agg#0=[COUNT() FILTER $3],
agg#1=[COUNT() FILTER $4])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[=($3, 0)],
$f4=[=($3, 1)])
+ * LogicalUnion(all=[true])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[0])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[1])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * @see CoreRules#MINUS_TO_DISTINCT
+ */
+@Value.Enclosing
+public class MinusToDistinctRule
+extends RelRule
+implements TransformationRule {
+
+ protected MinusToDistinctRule(Config config) {
+super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public MinusToDistinctRule(Class minusClass,
+ RelBuilderFactory relBuilderFactory) {
+
this(MinusToDistinctRule.Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+.as(MinusToDistinctRule.Config.class)
+.withOperandFor(minusClass));
+ }
+
+ @Override public void onMatch(RelOptRuleCall call) {
+Minus minus = call.rel(0);
+
+if (minus.all) {
+ // Nothing we can do
+ return;
+}
+
+final RelOptCluster cluster = minus.getCluster();
+final RelBuilder relBuilder = call.builder();
+final RexBuilder rexBuilder = cluster.getRexBuilder();
+final int branchCount = minus.getInputs().size();
+
+// For each child branch in minus,add a column which indicates branch index
+//
+// e.g. select EMPNO from emp -> select EMPNO,0 from emp
+// 0 indicates that it comes from minus the first child branch
+for (int i = 0; i < branchCount; i++) {
+ relBuilder.push(minus.getInput(i));
+ relBuilder.projectPlus(relBuilder.literal(new BigDecimal(i)));
Review Comment:
Nit: since this is a counter for the branch numbers, maybe `BigDecimal` is
an overkill?
EDIT: never mind, it's implemented the same way in `IntersectToDistinctRule`
so let's leave it this way
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3391: [CALCITE-5933] Add SAFE_DIVIDE function (enabled in BigQuery library)
sonarcloud[bot] commented on PR #3391: URL: https://github.com/apache/calcite/pull/3391#issuecomment-1701327621 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3391) [](https://sonarcloud.io/project/issues?id=apache_calcite=3391=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3391=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3391=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3391=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3391=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3391=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3391=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3391=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3391=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3391=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3391=false=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3391=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3391=new_coverage=list) [96.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3391=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3391=new_duplicated_lines_density=list) [24.4% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3391=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
[GitHub] [calcite] asolimando commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
asolimando commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311847209
##
core/src/main/java/org/apache/calcite/rel/rules/MinusToDistinctRule.java:
##
@@ -0,0 +1,184 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Minus;
+import org.apache.calcite.rel.logical.LogicalMinus;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+
+import org.immutables.value.Value;
+
+import java.math.BigDecimal;
+
+/**
+ * Planner rule that translates a distinct
+ * {@link org.apache.calcite.rel.core.Minus}
+ * (all = false)
+ * into a group of operators composed of
+ * {@link org.apache.calcite.rel.core.Union},
+ * {@link org.apache.calcite.rel.core.Aggregate},
+ * {@link org.apache.calcite.rel.core.Filter},etc.
+ *
+ * For example, the query plan
+
+ * {@code
+ * LogicalMinus(all=[false])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * will convert to
+ *
+ * {@code
+ * LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2])
+ *LogicalFilter(condition=[AND(>($3, 0), =($4, 0))])
+ * LogicalAggregate(group=[{0, 1, 2}], agg#0=[COUNT() FILTER $3],
agg#1=[COUNT() FILTER $4])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[=($3, 0)],
$f4=[=($3, 1)])
+ * LogicalUnion(all=[true])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[0])
+ * LogicalFilter(condition=[=($7, 10)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ *LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], $f3=[1])
+ * LogicalFilter(condition=[=($7, 20)])
+ *LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ * }
+ *
+ * @see CoreRules#MINUS_TO_DISTINCT
+ */
+@Value.Enclosing
+public class MinusToDistinctRule
+extends RelRule
+implements TransformationRule {
+
+ protected MinusToDistinctRule(Config config) {
+super(config);
+ }
+
+ @Deprecated // to be removed before 2.0
+ public MinusToDistinctRule(Class minusClass,
+ RelBuilderFactory relBuilderFactory) {
+
this(MinusToDistinctRule.Config.DEFAULT.withRelBuilderFactory(relBuilderFactory)
+.as(MinusToDistinctRule.Config.class)
+.withOperandFor(minusClass));
+ }
+
+ @Override public void onMatch(RelOptRuleCall call) {
+Minus minus = call.rel(0);
+
+if (minus.all) {
+ // Nothing we can do
+ return;
+}
+
+final RelOptCluster cluster = minus.getCluster();
+final RelBuilder relBuilder = call.builder();
+final RexBuilder rexBuilder = cluster.getRexBuilder();
+final int branchCount = minus.getInputs().size();
+
+// For each child branch in minus,add a column which indicates branch index
+//
+// e.g. select EMPNO from emp -> select EMPNO,0 from emp
+// 0 indicates that it comes from minus the first child branch
+for (int i = 0; i < branchCount; i++) {
+ relBuilder.push(minus.getInput(i));
+ relBuilder.projectPlus(relBuilder.literal(new BigDecimal(i)));
Review Comment:
Nit: since this is a counter for the branch numbers, maybe `BigDecimal` is
an overkill?
--
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
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
sonarcloud[bot] commented on PR #3367: URL: https://github.com/apache/calcite/pull/3367#issuecomment-1701296826 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3367) [](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3367=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3367=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3367=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3367=new_coverage=list) [93.7% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3367=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3367=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3367=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
[GitHub] [calcite] tanclary commented on pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
tanclary commented on PR #3367: URL: https://github.com/apache/calcite/pull/3367#issuecomment-1701291551 > Hi @tanclary @NobiGo ,I have already added the e2e test for my rule,if you have time, please help me to review again, thank you very much:) I think this looks good but I am less familiar with adding rules so I will defer to @NobiGo to see if he has other suggestions 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
[GitHub] [calcite] LakeShen commented on pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
LakeShen commented on PR #3367: URL: https://github.com/apache/calcite/pull/3367#issuecomment-1701286622 Hi @tanclary @NobiGo ,I have already added the e2e test for my rule,if you have time, please help me to review again, thank you very much:) -- 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] LakeShen commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
LakeShen commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311832848
##
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##
@@ -2699,6 +2699,35 @@ private void
checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
.check();
}
+ /** Tests {@link org.apache.calcite.rel.rules.MinusToDistinctRule},
Review Comment:
> In set-op.iq we need to support using this Rule to work. As far as I know
It didn't. But we can try to read
`https://github.com/apache/calcite/blob/main/core/src/test/java/org/apache/calcite/test/JdbcTest.java#L3749C14-L3749C27`.
@LakeShen Can we add tests like this?
Hi @NobiGo ,I have added the test for this rule in JdbcTest,please help me
to review again, thank you very much
--
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] tanclary commented on a diff in pull request #3395: [CALCITE-5948] Explicit casting should be made if the type of an element in ARRAY/MAP not equals with the derived component type
tanclary commented on code in PR #3395:
URL: https://github.com/apache/calcite/pull/3395#discussion_r1311824276
##
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##
@@ -1309,6 +1311,64 @@ public static boolean isMeasure(SqlNode selectItem) {
return null;
}
+ /**
+ * When the array element does not equal with the array component type,
making explicit casting.
+ *
+ * @param componentType derived array component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForArrayConstructor(
+ RelDataType componentType, SqlOperatorBinding opBinding) {
+if (opBinding instanceof SqlCallBinding) {
+ requireNonNull(componentType, "array component type");
+ adjustTypeForMultisetConstructor(
+ componentType, componentType, (SqlCallBinding) opBinding);
+}
+ }
+
+ /**
+ * When the map key or value does not equal with the map component key type
or value type,
+ * making explicit casting.
+ *
+ * @param componentType derived map pair component type
+ * @param opBinding description of call
+ */
+ public static void adjustTypeForMapConstructor(
+ Pair componentType, SqlOperatorBinding
opBinding) {
+if (opBinding instanceof SqlCallBinding) {
+ requireNonNull(componentType.getKey(), "map key type");
+ requireNonNull(componentType.getValue(), "map value type");
+ adjustTypeForMultisetConstructor(
+ componentType.getKey(), componentType.getValue(), (SqlCallBinding)
opBinding);
+}
+ }
+
+ private static void adjustTypeForMultisetConstructor(
+ RelDataType evenType, RelDataType oddType, SqlCallBinding
sqlCallBinding) {
+SqlCall call = sqlCallBinding.getCall();
+List operandTypes = sqlCallBinding.collectOperandTypes();
+List operands = call.getOperandList();
+RelDataType elementType;
+for (int i = 0; i < operands.size(); i++) {
+ if (i % 2 == 0) {
+elementType = evenType;
+ } else {
+elementType = oddType;
+ }
+ if (operandTypes.get(i).equalsSansFieldNames(elementType)) {
Review Comment:
can this be rewritten as `if(!operandTypes.get(i)) { call.setOperand() }`
##
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##
@@ -1309,6 +1311,64 @@ public static boolean isMeasure(SqlNode selectItem) {
return null;
}
+ /**
+ * When the array element does not equal with the array component type,
making explicit casting.
Review Comment:
i think you can remove `with` and change making->make
--
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] sonarcloud[bot] commented on pull request #3405: [CALCITE-5961] Type inference of ARRAY_COMPACT is incorrect
sonarcloud[bot] commented on PR #3405: URL: https://github.com/apache/calcite/pull/3405#issuecomment-1701176858 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3405) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3405=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3405=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3405=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3405=new_coverage=list) [93.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3405=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3405=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3405=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
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3381: [CALCITE-5939] Support for rewriting different join types in UnifyRule
sonarcloud[bot] commented on PR #3381: URL: https://github.com/apache/calcite/pull/3381#issuecomment-1701001311 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3381) [](https://sonarcloud.io/project/issues?id=apache_calcite=3381=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3381=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3381=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3381=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3381=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3381=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3381=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3381=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3381=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3381=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3381=false=CODE_SMELL) [19 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3381=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3381=new_coverage=list) [86.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3381=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3381=new_duplicated_lines_density=list) [7.9% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3381=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
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
sonarcloud[bot] commented on PR #3367: URL: https://github.com/apache/calcite/pull/3367#issuecomment-1700976207 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3367) [](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3367=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3367=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3367=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3367=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3367=new_coverage=list) [93.7% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3367=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3367=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3367=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
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3405: [CALCITE-5961] Type inference of ARRAY_COMPACT is incorrect
sonarcloud[bot] commented on PR #3405: URL: https://github.com/apache/calcite/pull/3405#issuecomment-1700971750 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3405) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3405=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3405=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3405=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3405=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3405=new_coverage=list) [93.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3405=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3405=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3405=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
[GitHub] [calcite] LakeShen commented on a diff in pull request #3396: [CALCITE-5940] Add the Rules to optimize Limit
LakeShen commented on code in PR #3396:
URL: https://github.com/apache/calcite/pull/3396#discussion_r1311555648
##
core/src/main/java/org/apache/calcite/rel/rules/SortMergeRule.java:
##
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.core.Sort;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.tools.RelBuilder;
+
+import com.google.common.collect.ImmutableList;
+
+import org.immutables.value.Value;
+
+/**
+ * This rule try to merge the double {@link Sort},one is Limit semantics,
+ * another sort is Limit or TOPN semantics.
+ *
+ * It generally used with the {@link SortProjectTransposeRule} rule.
+ *
+ * For example:
+ * {@code
+ * select
+ * concat('-', N_REGIONKEY) from
+ * (
+ * select
+ * *
+ * from nation limit 1) limit 10}
+ *
+ *
+ * will convert to
+ * {@code
+ * select
+ * concat('-', N_REGIONKEY)
+ * from
+ * nation limit 10
+ * }
+ *
+ * The sql :
+ * {@code
+ * select concat('-',N_REGIONKEY) from
+ * (SELECT * FROM nation order BY N_REGIONKEY DESC LIMIT 1) limit 10
+ * }
+ *
+ * will convert to
+ * {@code
+ * SELECT concat('-',N_REGIONKEY) FROM nation order BY N_REGIONKEY DESC LIMIT
10
+ * }
+ *
+ * In the future,we could also extend other sort merge logic in this rule.
+ */
+@Value.Enclosing
+public class SortMergeRule
+extends RelRule
+implements TransformationRule {
+
+ protected SortMergeRule(final SortMergeRule.Config config) {
+super(config);
+ }
+
+ @Override public void onMatch(final RelOptRuleCall call) {
+config.matchHandler().accept(this, call);
+ }
+
+ private static void limitMerge(SortMergeRule rule,
+ RelOptRuleCall call) {
+final Sort sort = call.rel(0);
+final Sort child = call.rel(1);
Review Comment:
Hi @kgyrtkirk ,thank you so much for your review advice,I will take your
advice later.
--
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] ILuffZhe commented on a diff in pull request #3387: [CALCITE-5911] Add REGEXP_EXTRACT_ALL function (enabled in BigQuery library)
ILuffZhe commented on code in PR #3387:
URL: https://github.com/apache/calcite/pull/3387#discussion_r1311531357
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -4675,6 +4675,29 @@ private static void checkIf(SqlOperatorFixture f) {
f.checkQuery("select regexp_extract('a9cadca5c4aecghi', 'a[0-9]c', 1, 3)");
}
+ @Test void testRegexpExtractAllFunc() {
+final SqlOperatorFixture f =
+
fixture().setFor(SqlLibraryOperators.REGEXP_EXTRACT_ALL).withLibrary(SqlLibrary.BIG_QUERY);
+
+f.checkScalar("regexp_extract_all('a9cadca5c4aecghi', 'a[0-9]c')", "[a9c,
a5c]", "CHAR(16) "
++ "NOT NULL ARRAY NOT NULL");
+f.checkScalar("regexp_extract_all('abcde', '.')", "[a, b, c, d, e]",
"CHAR(5) NOT NULL ARRAY "
++ "NOT NULL");
+f.checkScalar("regexp_extract_all('abcadcabcaecghi', 'ac')", "[]",
"CHAR(15) NOT NULL ARRAY "
++ "NOT NULL");
+f.checkScalar("regexp_extract_all('f...@bar.com, f...@gmail.com,
f...@outlook.com', "
++ "'@[a-zA-Z0-9-]+\\.[a-zA-Z0-9-.]+')", "[@bar.com, @gmail.com,
@outlook.com]", "CHAR(43) "
++ "NOT NULL ARRAY NOT NULL");
+
+f.checkNull("regexp_extract_all('abc def ghi', cast(null as varchar))");
Review Comment:
In your tests, I noticed that either the value or the regex is null, the
function returns null.
What if we use `ReturnTypes.ARG0_NULLABLE` for this func?
--
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] LakeShen commented on a diff in pull request #3367: [CALCITE-5889] Add the RelRule that converts Minus into UNION ALL..GROUP BY...WHERE
LakeShen commented on code in PR #3367:
URL: https://github.com/apache/calcite/pull/3367#discussion_r1311499852
##
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##
@@ -2699,6 +2699,35 @@ private void
checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
.check();
}
+ /** Tests {@link org.apache.calcite.rel.rules.MinusToDistinctRule},
Review Comment:
@NobiGo @herunkang2018 ,thank you very much for your advice,I will take it
later.
--
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] jiefei30 commented on pull request #3399: [CALCITE-5962] Support parse Spark-style syntax LEFT ANTI JOIN in Babel parser
jiefei30 commented on PR #3399: URL: https://github.com/apache/calcite/pull/3399#issuecomment-1700836979 > Hi @jiefei30, I think you should consider this PR[#3291], LEFT ANTI JOIN should not refer right-hand table. @macroguo-ghy ok, thanks for your suggestion, I'll check 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
[GitHub] [calcite] macroguo-ghy commented on pull request #3399: [CALCITE-5962] Support parse Spark-style syntax LEFT ANTI JOIN in Babel parser
macroguo-ghy commented on PR #3399: URL: https://github.com/apache/calcite/pull/3399#issuecomment-1700738206 Hi @jiefei30, I think you should consider this PR[https://github.com/apache/calcite/pull/3291], LEFT ANTI JOIN should not refer right-hand table. -- 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-5965] Avoid unnecessary String concatenations in the RexFieldAccess constructor to improve the performance
This is an automated email from the ASF dual-hosted git repository.
rubenql 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 267d633165 [CALCITE-5965] Avoid unnecessary String concatenations in
the RexFieldAccess constructor to improve the performance
267d633165 is described below
commit 267d63316511289ebe91079714e43b4e2d2ba085
Author: Thomas Rebele
AuthorDate: Tue Aug 29 15:21:37 2023 +0200
[CALCITE-5965] Avoid unnecessary String concatenations in the
RexFieldAccess constructor to improve the performance
---
core/src/main/java/org/apache/calcite/rex/RexFieldAccess.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/core/src/main/java/org/apache/calcite/rex/RexFieldAccess.java
b/core/src/main/java/org/apache/calcite/rex/RexFieldAccess.java
index 886f98f0e5..0f09672f38 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexFieldAccess.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexFieldAccess.java
@@ -75,7 +75,7 @@ public class RexFieldAccess extends RexNode {
Preconditions.checkArgument(
fieldIdx >= 0 && fieldIdx < exprType.getFieldList().size()
&& exprType.getFieldList().get(fieldIdx).equals(field),
-"Field " + field + " does not exist for expression " + expr);
+"Field %s does not exist for expression %s", field, expr);
}
public RelDataTypeField getField() {
[GitHub] [calcite] rubenada merged pull request #3401: [CALCITE-5965] Avoid unnecessary String concatenations in the RexFieldAccess constructor to improve the performance
rubenada merged PR #3401: URL: https://github.com/apache/calcite/pull/3401 -- 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] sonarcloud[bot] commented on pull request #3401: [CALCITE-5965] Avoid unnecessary String concatenations in the RexFieldAccess constructor to improve the performance
sonarcloud[bot] commented on PR #3401: URL: https://github.com/apache/calcite/pull/3401#issuecomment-1700683135 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3401) [](https://sonarcloud.io/project/issues?id=apache_calcite=3401=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3401=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3401=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3401=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3401=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3401=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3401=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3401=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3401=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3401=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3401=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3401=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3401=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_calcite=3401=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3401=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
[GitHub] [calcite] zstan commented on pull request #3393: [CALCITE-5950] Default column constraint is erroneously processed
zstan commented on PR #3393: URL: https://github.com/apache/calcite/pull/3393#issuecomment-1700642138 @julianhyde thanks for review, all done, i don`t know the reason of 1 erroneous check 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
