You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/12/12 08:42:19 UTC
[groovy] branch master updated: Support smart inner join that has
better readability
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 0555ed0 Support smart inner join that has better readability
0555ed0 is described below
commit 0555ed022eb6f2abf41a260e5e2fdc577ea8afa9
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sat Dec 12 16:41:51 2020 +0800
Support smart inner join that has better readability
---
.../org/apache/groovy/ginq/dsl/GinqAstBuilder.java | 8 ++-
.../apache/groovy/ginq/dsl/GinqAstOptimizer.groovy | 2 +-
.../groovy/ginq/dsl/expression/JoinExpression.java | 13 ++--
.../ginq/provider/collection/GinqAstWalker.groovy | 72 +++++++++++++++-------
.../groovy-ginq/src/spec/doc/ginq-userguide.adoc | 8 +++
.../test/org/apache/groovy/ginq/GinqTest.groovy | 34 +++++++++-
6 files changed, 105 insertions(+), 32 deletions(-)
diff --git a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstBuilder.java b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstBuilder.java
index 1266b90..80f71af 100644
--- a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstBuilder.java
+++ b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstBuilder.java
@@ -386,7 +386,9 @@ public class GinqAstBuilder extends CodeVisitorSupport implements SyntaxErrorRep
private static final String KW_ORDERBY = "orderby";
private static final String KW_LIMIT = "limit";
private static final String KW_SELECT = "select";
- private static final Set<String> KEYWORD_SET = new HashSet<>(Arrays.asList(
- KW_FROM, "innerjoin", "innerhashjoin", "leftjoin", "lefthashjoin", "rightjoin", "righthashjoin", "fulljoin", "fullhashjoin", "crossjoin",
- KW_WHERE, KW_ON, KW_HAVING, KW_EXISTS, KW_GROUPBY, KW_ORDERBY, KW_LIMIT, KW_SELECT));
+ private static final Set<String> KEYWORD_SET = new HashSet<>();
+ static {
+ KEYWORD_SET.addAll(Arrays.asList(KW_FROM, KW_WHERE, KW_ON, KW_HAVING, KW_EXISTS, KW_GROUPBY, KW_ORDERBY, KW_LIMIT, KW_SELECT));
+ KEYWORD_SET.addAll(JoinExpression.JOIN_NAME_LIST);
+ }
}
diff --git a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstOptimizer.groovy b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstOptimizer.groovy
index 1bef2c4..acb5cfa 100644
--- a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstOptimizer.groovy
+++ b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/GinqAstOptimizer.groovy
@@ -116,7 +116,7 @@ class GinqAstOptimizer extends GinqAstBaseVisitor {
List<DataSourceExpression> optimizingDataSourceExpressionList = []
optimizingDataSourceExpressionList << ginqExpression.fromExpression
for (JoinExpression joinExpression : ginqExpression.joinExpressionList) {
- if (joinExpression.innerJoin || joinExpression.innerHashJoin) {
+ if (joinExpression.smartInnerJoin || joinExpression.innerJoin || joinExpression.innerHashJoin) {
optimizingDataSourceExpressionList << joinExpression
} else if (joinExpression.leftJoin || joinExpression.leftHashJoin) {
break
diff --git a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/expression/JoinExpression.java b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/expression/JoinExpression.java
index 8517a3a..7baa505 100644
--- a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/expression/JoinExpression.java
+++ b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/dsl/expression/JoinExpression.java
@@ -30,13 +30,14 @@ import java.util.List;
* @since 4.0.0
*/
public class JoinExpression extends DataSourceExpression implements DataSourceHolder {
- private static final String INNER_JOIN = "innerjoin";
- private static final String INNER_HASH_JOIN = "innerhashjoin";
+ public static final String SMART_INNER_JOIN = "join";
+ public static final String INNER_JOIN = "innerjoin";
+ public static final String INNER_HASH_JOIN = "innerhashjoin";
private static final String LEFT_JOIN = "leftjoin";
public static final String LEFT_HASH_JOIN = "lefthashjoin";
private static final String CROSS_JOIN = "crossjoin";
- private static final List<String> JOIN_NAME_LIST =
- Arrays.asList(INNER_JOIN, INNER_HASH_JOIN, LEFT_JOIN, LEFT_HASH_JOIN, "rightjoin", "righthashjoin", "fulljoin", "fullhashjoin", CROSS_JOIN);
+ public static final List<String> JOIN_NAME_LIST =
+ Arrays.asList(SMART_INNER_JOIN, INNER_JOIN, INNER_HASH_JOIN, LEFT_JOIN, LEFT_HASH_JOIN, "rightjoin", "righthashjoin", "fulljoin", "fullhashjoin", CROSS_JOIN);
private final String joinName;
private OnExpression onExpression;
@@ -55,6 +56,10 @@ public class JoinExpression extends DataSourceExpression implements DataSourceHo
return CROSS_JOIN.equals(joinName);
}
+ public boolean isSmartInnerJoin() {
+ return SMART_INNER_JOIN.equals(joinName);
+ }
+
public boolean isInnerJoin() {
return INNER_JOIN.equals(joinName);
}
diff --git a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/provider/collection/GinqAstWalker.groovy b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/provider/collection/GinqAstWalker.groovy
index 945735e..47662d5 100644
--- a/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/provider/collection/GinqAstWalker.groovy
+++ b/subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/provider/collection/GinqAstWalker.groovy
@@ -67,6 +67,7 @@ import org.codehaus.groovy.syntax.Token
import org.codehaus.groovy.syntax.Types
import org.objectweb.asm.Opcodes
+import java.util.function.Consumer
import java.util.stream.Collectors
import static groovy.lang.Tuple.tuple
@@ -308,10 +309,16 @@ class GinqAstWalker implements GinqAstVisitor<Expression>, SyntaxErrorReportable
List<Expression> argumentExpressionList = []
argumentExpressionList << constructFromMethodCallExpression(joinExpression.dataSourceExpr)
- if (joinExpression.joinName.toLowerCase().contains('hash')) {
+ def joinName = joinExpression.joinName
+
+ List<BinaryExpression> equalExpressionList = collectEqualExpressionForHashJoin(onExpression)
+ if (joinExpression.smartInnerJoin) {
+ joinName = equalExpressionList ? JoinExpression.INNER_HASH_JOIN : JoinExpression.INNER_JOIN
+ }
+
+ if (joinName.toLowerCase().contains('hash')) {
List<Expression> leftExpressionList = []
List<Expression> rightExpressionList = []
- List<BinaryExpression> equalExpressionList = []
if (onExpression.filterExpr !instanceof BinaryExpression) {
this.collectSyntaxError(
@@ -322,25 +329,15 @@ class GinqAstWalker implements GinqAstVisitor<Expression>, SyntaxErrorReportable
)
}
- onExpression.filterExpr.visit(new CodeVisitorSupport() {
- @Override
- void visitBinaryExpression(BinaryExpression expression) {
- if (Types.LOGICAL_AND == expression.operation.type) {
- super.visitBinaryExpression(expression)
- return
- } else if (Types.COMPARE_EQUAL == expression.operation.type) {
- equalExpressionList << expression
- return
- }
-
- GinqAstWalker.this.collectSyntaxError(
- new GinqSyntaxError(
- "`" + expression.operation.text + "` is not allowed in `on` clause of hash join",
- expression.getLineNumber(), expression.getColumnNumber()
- )
- )
- }
- })
+ if (!equalExpressionList) {
+ collectEqualExpressionForHashJoin(onExpression,
+ expression -> collectSyntaxError(
+ new GinqSyntaxError(
+ "`" + expression.operation.text + "` is not allowed in `on` clause of hash join",
+ expression.getLineNumber(), expression.getColumnNumber()
+ )
+ ))
+ }
equalExpressionList.forEach((BinaryExpression expression) -> {
collectHashJoinFields([expression.leftExpression, expression.rightExpression], joinExpression.aliasExpr.text, leftExpressionList, rightExpressionList)
@@ -373,7 +370,7 @@ class GinqAstWalker implements GinqAstVisitor<Expression>, SyntaxErrorReportable
MethodCallExpression resultMethodCallExpression
MethodCallExpression joinMethodCallExpression =
callX(receiver,
- joinExpression.joinName
+ joinName
.replace('join', 'Join')
.replace('hash', 'Hash'),
args(argumentExpressionList))
@@ -390,6 +387,37 @@ class GinqAstWalker implements GinqAstVisitor<Expression>, SyntaxErrorReportable
return resultMethodCallExpression
}
+ private List<BinaryExpression> collectEqualExpressionForHashJoin(OnExpression onExpression, Consumer<BinaryExpression> errorCollector=null) {
+ if (!onExpression) return Collections.emptyList()
+
+ List<BinaryExpression> equalExpressionList = []
+ boolean valid = true
+
+ onExpression.filterExpr.visit(new CodeVisitorSupport() {
+ @Override
+ void visitBinaryExpression(BinaryExpression expression) {
+ if (Types.LOGICAL_AND == expression.operation.type) {
+ super.visitBinaryExpression(expression)
+ return
+ } else if (Types.COMPARE_EQUAL == expression.operation.type) {
+ equalExpressionList << expression
+ return
+ }
+
+ valid = false
+ if (errorCollector) {
+ errorCollector.accept(expression)
+ }
+ }
+ })
+
+ if (!valid) {
+ equalExpressionList.clear()
+ }
+
+ return equalExpressionList
+ }
+
private void collectHashJoinFields(List<Expression> expressionList, String joinAliasName, List<Expression> leftExpressionList, List<Expression> rightExpressionList) {
expressionList.each {expression ->
List<Expression> foundVariableExpressionList = []
diff --git a/subprojects/groovy-ginq/src/spec/doc/ginq-userguide.adoc b/subprojects/groovy-ginq/src/spec/doc/ginq-userguide.adoc
index ed35316..c43089f 100644
--- a/subprojects/groovy-ginq/src/spec/doc/ginq-userguide.adoc
+++ b/subprojects/groovy-ginq/src/spec/doc/ginq-userguide.adoc
@@ -165,6 +165,14 @@ More data sources for GINQ could be specified by join clauses.
[source, sql]
----
+include::../test/org/apache/groovy/ginq/GinqTest.groovy[tags=ginq_joining_10,indent=0]
+----
+[NOTE]
+`join` is preferred over `innerjoin` and `innerhashjoin` as it has better readability,
+and it is smart enough to choose the correct concrete join(i.e. `innerjoin` or `innerhashjoin`) by its `on` clause.
+
+[source, sql]
+----
include::../test/org/apache/groovy/ginq/GinqTest.groovy[tags=ginq_joining_01,indent=0]
----
diff --git a/subprojects/groovy-ginq/src/spec/test/org/apache/groovy/ginq/GinqTest.groovy b/subprojects/groovy-ginq/src/spec/test/org/apache/groovy/ginq/GinqTest.groovy
index 69aabdf..40b28ef 100644
--- a/subprojects/groovy-ginq/src/spec/test/org/apache/groovy/ginq/GinqTest.groovy
+++ b/subprojects/groovy-ginq/src/spec/test/org/apache/groovy/ginq/GinqTest.groovy
@@ -257,6 +257,30 @@ class GinqTest {
}
@Test
+ void "testGinq - from smartinnerjoin select - 1"() {
+ assertGinqScript '''
+ assert [[1, 1], [3, 3]] == GQ {
+// tag::ginq_joining_10[]
+ from n1 in [1, 2, 3]
+ join n2 in [1, 3] on n1 == n2
+ select n1, n2
+// end::ginq_joining_10[]
+ }.toList()
+ '''
+ }
+
+ @Test
+ void "testGinq - from smartinnerjoin select - 2"() {
+ assertGinqScript '''
+ assert [[1, 3], [2, 3]] == GQ {
+ from n1 in [1, 2, 3]
+ join n2 in [1, 3] on n2 > n1
+ select n1, n2
+ }.toList()
+ '''
+ }
+
+ @Test
void "testGinq - from innerjoin select - 1"() {
assertGinqScript '''
def nums1 = [1, 2, 3]
@@ -4260,8 +4284,14 @@ class GinqTest {
}
private static void assertGinqScript(String script) {
- String newScript = script.replaceAll(/\bGQ\s*[{]/, 'GQ(optimize:false) {')
- List<String> scriptList = [newScript, script]
+ String deoptimizedScript = script.replaceAll(/\bGQ\s*[{]/, 'GQ(optimize:false) {')
+ List<String> scriptList = [deoptimizedScript, script]
+
+ if (-1 != deoptimizedScript.indexOf('innerjoin') || -1 != deoptimizedScript.indexOf('innerhashjoin')) {
+ String smartJoinScript = deoptimizedScript.replaceAll('innerjoin|innerhashjoin', 'join')
+ scriptList << smartJoinScript
+ }
+
scriptList.each { String c ->
assertScript(c)
}