Skip to content

Commit ffc3e84

Browse files
jnmugerwaRongrong Zhong
authored andcommitted
Add base rewriter classes with specific implementation for rewriting ORDER BY anti-pattern
[maven-release-plugin] prepare release 0.1 [maven-release-plugin] prepare for next development iteration
1 parent 14b2319 commit ffc3e84

File tree

8 files changed

+558
-0
lines changed

8 files changed

+558
-0
lines changed

parser/src/main/java/com/facebook/coresql/parser/AstNode.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ public final AstNode GetFirstChildOfKind(int kind)
7575
return null;
7676
}
7777

78+
public final boolean hasChildOfKind(int kind)
79+
{
80+
return GetFirstChildOfKind(kind) != null;
81+
}
82+
7883
public Location getLocation()
7984
{
8085
return new Location(beginToken.beginLine, beginToken.beginColumn, endToken.endLine, endToken.endColumn);

pom.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
<modules>
2525
<module>parser</module>
2626
<module>linter</module>
27+
<module>rewriter</module>
2728
</modules>
2829

2930
<build>

rewriter/pom.xml

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
3+
<modelVersion>4.0.0</modelVersion>
4+
5+
<parent>
6+
<groupId>com.facebook.presto</groupId>
7+
<artifactId>presto-coresql</artifactId>
8+
<version>0.2-SNAPSHOT</version>
9+
</parent>
10+
11+
<artifactId>presto-coresql-rewriter</artifactId>
12+
<name>presto-coresql-rewriter</name>
13+
14+
<properties>
15+
<air.main.basedir>${project.parent.basedir}</air.main.basedir>
16+
<maven.compiler.source>1.6</maven.compiler.source>
17+
<maven.compiler.target>1.6</maven.compiler.target>
18+
</properties>
19+
20+
<dependencies>
21+
<dependency>
22+
<groupId>junit</groupId>
23+
<artifactId>junit</artifactId>
24+
<version>4.12</version>
25+
<scope>test</scope>
26+
</dependency>
27+
28+
<dependency>
29+
<groupId>org.testng</groupId>
30+
<artifactId>testng</artifactId>
31+
<scope>test</scope>
32+
</dependency>
33+
34+
<dependency>
35+
<groupId>com.facebook.presto</groupId>
36+
<artifactId>presto-coresql-parser</artifactId>
37+
<version>0.2-SNAPSHOT</version>
38+
</dependency>
39+
40+
<dependency>
41+
<groupId>com.fasterxml.jackson.core</groupId>
42+
<artifactId>jackson-annotations</artifactId>
43+
</dependency>
44+
</dependencies>
45+
<build>
46+
<plugins>
47+
<plugin>
48+
<groupId>org.codehaus.mojo</groupId>
49+
<artifactId>build-helper-maven-plugin</artifactId>
50+
<executions>
51+
<execution>
52+
<phase>generate-sources</phase>
53+
<goals>
54+
<goal>add-source</goal>
55+
</goals>
56+
<configuration>
57+
<sources>
58+
<source>${project.build.directory}/generated-sources</source>
59+
</sources>
60+
</configuration>
61+
</execution>
62+
</executions>
63+
</plugin>
64+
65+
<plugin>
66+
<groupId>com.facebook.presto</groupId>
67+
<artifactId>presto-maven-plugin</artifactId>
68+
<version>0.3</version>
69+
<extensions>true</extensions>
70+
</plugin>
71+
72+
<plugin>
73+
<groupId>org.apache.maven.plugins</groupId>
74+
<artifactId>maven-shade-plugin</artifactId>
75+
<version>3.1.1</version>
76+
</plugin>
77+
78+
<plugin>
79+
<groupId>org.skife.maven</groupId>
80+
<artifactId>really-executable-jar-maven-plugin</artifactId>
81+
<version>1.0.5</version>
82+
</plugin>
83+
84+
<plugin>
85+
<groupId>org.apache.maven.plugins</groupId>
86+
<artifactId>maven-antrun-plugin</artifactId>
87+
<version>1.8</version>
88+
</plugin>
89+
90+
<plugin>
91+
<groupId>io.airlift.maven.plugins</groupId>
92+
<artifactId>sphinx-maven-plugin</artifactId>
93+
<version>2.1</version>
94+
</plugin>
95+
96+
<plugin>
97+
<groupId>org.apache.maven.plugins</groupId>
98+
<artifactId>maven-enforcer-plugin</artifactId>
99+
<configuration>
100+
<rules>
101+
<requireUpperBoundDeps>
102+
<excludes combine.children="append">
103+
<!-- TODO: fix this in Airlift resolver -->
104+
<exclude>org.alluxio:alluxio-shaded-client</exclude>
105+
<exclude>org.codehaus.plexus:plexus-utils</exclude>
106+
<exclude>com.google.guava:guava</exclude>
107+
</excludes>
108+
</requireUpperBoundDeps>
109+
</rules>
110+
</configuration>
111+
</plugin>
112+
113+
<plugin>
114+
<groupId>org.apache.maven.plugins</groupId>
115+
<artifactId>maven-release-plugin</artifactId>
116+
<configuration>
117+
<preparationGoals>clean verify -DskipTests</preparationGoals>
118+
</configuration>
119+
</plugin>
120+
121+
<plugin>
122+
<groupId>org.apache.maven.plugins</groupId>
123+
<artifactId>maven-compiler-plugin</artifactId>
124+
<configuration combine.children="append">
125+
<fork>true</fork>
126+
<compilerArgs>
127+
<arg>-verbose</arg>
128+
<arg>-J-Xss100M</arg>
129+
</compilerArgs>
130+
</configuration>
131+
</plugin>
132+
133+
<plugin>
134+
<groupId>org.apache.maven.plugins</groupId>
135+
<artifactId>maven-surefire-plugin</artifactId>
136+
<configuration combine.children="append">
137+
<includes>
138+
<include>**/*.java</include>
139+
<include>target/**/*.java</include>
140+
<include>**/Benchmark*.java</include>
141+
</includes>
142+
<excludes>
143+
<exclude>**/*jmhTest*.java</exclude>
144+
<exclude>**/*jmhType*.java</exclude>
145+
</excludes>
146+
</configuration>
147+
</plugin>
148+
149+
<!-- Always build a jar with the test classes -->
150+
<plugin>
151+
<groupId>org.apache.maven.plugins</groupId>
152+
<artifactId>maven-jar-plugin</artifactId>
153+
<configuration>
154+
<!-- do not build an empty jar if the project is
155+
e.g. a pom project -->
156+
<skipIfEmpty>true</skipIfEmpty>
157+
<archive>
158+
<manifest>
159+
<addDefaultImplementationEntries>true</addDefaultImplementationEntries>
160+
<addDefaultSpecificationEntries>true</addDefaultSpecificationEntries>
161+
<addClasspath>false</addClasspath>
162+
</manifest>
163+
</archive>
164+
</configuration>
165+
</plugin>
166+
</plugins>
167+
<pluginManagement>
168+
<plugins>
169+
<plugin>
170+
<groupId>org.gaul</groupId>
171+
<artifactId>modernizer-maven-plugin</artifactId>
172+
<version>2.1.0</version>
173+
<configuration>
174+
<javaVersion>1.8</javaVersion>
175+
</configuration>
176+
</plugin>
177+
</plugins>
178+
</pluginManagement>
179+
</build>
180+
</project>
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
15+
package com.facebook.coresql.rewriter;
16+
17+
import com.facebook.coresql.parser.AstNode;
18+
import com.facebook.coresql.parser.OrderByClause;
19+
import com.facebook.coresql.parser.QuerySpecification;
20+
import com.facebook.coresql.parser.SimpleNode;
21+
import com.facebook.coresql.parser.SqlParserDefaultVisitor;
22+
import com.facebook.coresql.parser.Subquery;
23+
import com.facebook.coresql.parser.Unparser;
24+
25+
import java.util.HashSet;
26+
import java.util.Set;
27+
28+
import static com.facebook.coresql.parser.ParserHelper.parseStatement;
29+
import static com.facebook.coresql.parser.SqlParserTreeConstants.JJTINSERT;
30+
import static com.facebook.coresql.parser.SqlParserTreeConstants.JJTLIMITCLAUSE;
31+
import static com.facebook.coresql.parser.SqlParserTreeConstants.JJTORDERBYCLAUSE;
32+
import static com.facebook.coresql.parser.SqlParserTreeConstants.JJTTABLEDEFINITION;
33+
import static java.util.Objects.requireNonNull;
34+
35+
public class OrderByRewriter
36+
extends Rewriter
37+
{
38+
PatternMatcher<Set<AstNode>> matcher;
39+
public Set<AstNode> patternMatchedNodes;
40+
private static final String REWRITE_NAME = "ORDER BY without LIMIT";
41+
42+
public OrderByRewriter()
43+
{
44+
this.matcher = new OrderByPatternMatcher();
45+
this.patternMatchedNodes = new HashSet<>();
46+
}
47+
48+
@Override
49+
public boolean rewritePatternIsPresent(String sql)
50+
{
51+
AstNode root = requireNonNull(parseStatement(sql));
52+
patternMatchedNodes = matcher.matchPattern(root);
53+
return !patternMatchedNodes.isEmpty();
54+
}
55+
56+
@Override
57+
public RewriteResult rewrite(String originalSql)
58+
{
59+
AstNode root = requireNonNull(parseStatement(originalSql));
60+
patternMatchedNodes = matcher.matchPattern(root);
61+
String rewrittenSql = Unparser.unparse(root, this);
62+
return new RewriteResult(REWRITE_NAME, originalSql, rewrittenSql);
63+
}
64+
65+
@Override
66+
public void visit(OrderByClause node, Void data)
67+
{
68+
if (patternMatchedNodes.contains(node)) {
69+
unparseUpto(node);
70+
moveToEndOfNode(node);
71+
}
72+
}
73+
74+
private static class OrderByPatternMatcher
75+
extends SqlParserDefaultVisitor
76+
implements PatternMatcher<Set<AstNode>>
77+
{
78+
private final Set<AstNode> matchedNodes;
79+
private int depth;
80+
private static final int MINIMUM_SUBQUERY_DEPTH = 2; // Past this depth, all queries we encounter are subqueries
81+
82+
public OrderByPatternMatcher()
83+
{
84+
this.matchedNodes = new HashSet<>();
85+
}
86+
87+
@Override
88+
public Set<AstNode> matchPattern(AstNode node)
89+
{
90+
requireNonNull(node, "AST passed to pattern matcher was null");
91+
node.jjtAccept(this, null);
92+
return matchedNodes;
93+
}
94+
95+
/**
96+
* Checks for a specific pattern in the AST this pattern matcher is traversing:
97+
* 1. Query has an ORDER BY
98+
* 2. Query does not have a LIMIT
99+
* 3. The query is a subquery OR the result of the query is used to CREATE a table or INSERT into a table
100+
*
101+
* @param node The node we're currently visiting
102+
* @return True if the pattern is matched at the current node, else false
103+
*/
104+
private boolean hasOrderByWithNoLimit(AstNode node)
105+
{
106+
if (depth > MINIMUM_SUBQUERY_DEPTH) {
107+
return hasOrderByAndDoesNotHaveLimit(node);
108+
}
109+
return hasOrderByAndDoesNotHaveLimit(node) && isUsedInInsertOrCreate(node); // Non-subquery check
110+
}
111+
112+
private boolean hasOrderByAndDoesNotHaveLimit(AstNode node)
113+
{
114+
return node.hasChildOfKind(JJTORDERBYCLAUSE) && !node.hasChildOfKind(JJTLIMITCLAUSE);
115+
}
116+
117+
private boolean isUsedInInsertOrCreate(AstNode node)
118+
{
119+
return (node.jjtGetParent().getId() == JJTTABLEDEFINITION || node.jjtGetParent().getId() == JJTINSERT);
120+
}
121+
122+
@Override
123+
public void defaultVisit(SimpleNode node, Void data)
124+
{
125+
++depth;
126+
node.childrenAccept(this, data);
127+
--depth;
128+
}
129+
130+
@Override
131+
public void visit(QuerySpecification node, Void data)
132+
{
133+
if (hasOrderByWithNoLimit(node)) {
134+
matchedNodes.add(node.GetFirstChildOfKind(JJTORDERBYCLAUSE));
135+
}
136+
defaultVisit(node, data);
137+
}
138+
139+
@Override
140+
public void visit(Subquery node, Void data)
141+
{
142+
if (hasOrderByWithNoLimit(node)) {
143+
matchedNodes.add(node.GetFirstChildOfKind(JJTORDERBYCLAUSE));
144+
}
145+
defaultVisit(node, data);
146+
}
147+
}
148+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
15+
package com.facebook.coresql.rewriter;
16+
17+
import com.facebook.coresql.parser.AstNode;
18+
19+
public interface PatternMatcher<T>
20+
{
21+
/**
22+
* Searches for a pattern in an Abstract Syntax Tree (AST).
23+
*
24+
* @param root root of the AST
25+
* @return T An arbitrary data structure containing information found during the pattern matching routine
26+
* Example: T is a Set<AstNode> which contains nodes that match the given pattern.
27+
*/
28+
T matchPattern(AstNode root);
29+
}

0 commit comments

Comments
 (0)