Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not capitalize reserved words when used as column names #60

Merged
merged 3 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
/*.iml
*.jj
target
/.vscode/
29 changes: 29 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
<junit.version>4.13.2</junit.version>
<spotless.version>2.42.0</spotless.version>
<exec-maven.version>3.1.1</exec-maven.version>
<build-helper-maven-plugin.version>3.5.0</build-helper-maven-plugin.version>
</properties>
<dependencies>
<dependency>
Expand Down Expand Up @@ -69,6 +70,12 @@
<artifactId>truth</artifactId>
<version>${truth.version}</version>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</exclusion>
</exclusions>
</dependency>
</dependencies>
<build>
Expand Down Expand Up @@ -100,6 +107,8 @@
<version>${exec-maven.version}</version>
<executions>
<execution>
<!-- For VSCode to regenerate sources on change -->
<?m2e execute onConfiguration,onIncremental?>
<id>jjt-2-concat</id>
<goals>
<goal>exec</goal>
Expand Down Expand Up @@ -189,6 +198,26 @@
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>build-helper-maven-plugin</artifactId>
<version>${build-helper-maven-plugin.version}</version>
<executions>
<execution>
<id>add-source</id>
<goals>
<goal>add-source</goal>
</goals>
<phase>generate-sources</phase>
<configuration>
<sources>
<source>${project.build.directory}/generated-sources/jjtree</source>
</sources>
<skipAddSourceIfMissing></skipAddSourceIfMissing>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,20 @@ private ASTTreeUtils() {}
* spacing and capitalization of tokens.
*/
public static String tokensToString(Token firstToken, Token lastToken) {
return tokensToString(firstToken, lastToken, true);
}

/**
* Generate the original parsed text between the 2 specified tokens, normalizing the text with
* spacing and optional capitalization of reserved words.
*/
public static String tokensToString(
Token firstToken, Token lastToken, boolean upperCaseReserved) {
StringBuilder sb = new StringBuilder();
Token t = firstToken;
while (t != lastToken) {
String tok = t.toString();
sb.append(isReservedWord(tok) ? tok.toUpperCase() : tok);
sb.append(isReservedWord(tok) && upperCaseReserved ? tok.toUpperCase() : tok);

if (t.next != null
&& !t.next.toString().equals(",")
Expand All @@ -91,15 +100,23 @@ public static String tokensToString(Token firstToken, Token lastToken) {
}
// append last token
String tok = t.toString();
sb.append(isReservedWord(tok) ? tok.toUpperCase() : tok);
sb.append(isReservedWord(tok) && upperCaseReserved ? tok.toUpperCase() : tok);
return sb.toString();
}

/**
* Generate the original parsed text of the node, normalizing the text with spacing and
* capitalization of tokens.
* capitalization of reserved words.
*/
public static String tokensToString(SimpleNode node) {
return tokensToString(node.jjtGetFirstToken(), node.jjtGetLastToken());
return tokensToString(node, true);
}

/**
* Generate the original parsed text of the node, normalizing the text with spacing and optional
* capitalization of reserved words.
*/
public static String tokensToString(SimpleNode node, boolean upperCaseReserved) {
return tokensToString(node.jjtGetFirstToken(), node.jjtGetLastToken(), upperCaseReserved);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public List<String> getConstrainedColumnNames() {

private List<String> identifierListToStringList(ASTidentifier_list idList) {
return Arrays.stream(idList.children)
.map(o -> ASTTreeUtils.tokensToString((ASTidentifier) o))
.map(o -> ASTTreeUtils.tokensToString((ASTidentifier) o, false))
.collect(Collectors.toList());
}

Expand All @@ -57,7 +57,7 @@ public String getReferencedTableName() {
if (children[0] instanceof ASTconstraint_name) {
child++;
}
return ASTTreeUtils.tokensToString((ASTreferenced_table) children[child]);
return ASTTreeUtils.tokensToString((ASTreferenced_table) children[child], false);
}

public List<String> getReferencedColumnNames() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ public ASTgeneration_clause(DdlParser p, int id) {

public String toString() {
final ASTexpression exp = (ASTexpression) children[0];
return " AS ( " + exp.toString() + " ) STORED";
return "AS ( " + exp.toString() + " ) STORED";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

package com.google.cloud.solutions.spannerddl.parser;

import com.google.cloud.solutions.spannerddl.diff.ASTTreeUtils;

/** Abstract Syntax Tree parser object for "key_part" token */
public class ASTkey_part extends SimpleNode {

Expand All @@ -35,13 +33,11 @@ public String toString() {
return jjtGetFirstToken().toString();
}
if (children.length == 1) {
return ASTTreeUtils.tokensToString((ASTpath) children[0])
+ " ASC"; // key name without direction ;

return ((ASTpath) children[0]).toString() + " ASC"; // key name without direction ;
} else {
// key name and ASC/DESC
return ASTTreeUtils.tokensToString((ASTpath) children[0])
+ " "
+ children[1].toString().toUpperCase();
return ((ASTpath) children[0]).toString() + " " + children[1].toString().toUpperCase();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ public ASTname(DdlParser p, int id) {

@Override
public String toString() {
return ASTTreeUtils.tokensToString(this);
return ASTTreeUtils.tokensToString(this, false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ public ASTpath(DdlParser p, int id) {

@Override
public String toString() {
return ASTTreeUtils.tokensToString(this);
return ASTTreeUtils.tokensToString(this, false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ public ASTstored_column(DdlParser p, int id) {

@Override
public String toString() {
return children[0].toString();
return ((ASTpath) children[0]).toString();
}
}
2 changes: 1 addition & 1 deletion src/main/jjtree-sources/DdlParser.head
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ options {
}
PARSER_BEGIN(DdlParser)
package com.google.cloud.solutions.spannerddl.parser;
import java.io.InputStream;import java.io.StringReader;import java.util.*;
import java.io.StringReader;

public class DdlParser {
public static ASTddl_statement parseDdlStatement(String in)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package com.google.cloud.solutions.spannerddl.diff;

import static com.google.cloud.solutions.spannerddl.diff.DdlDiff.ALLOW_DROP_STATEMENTS_OPT;
import static com.google.cloud.solutions.spannerddl.diff.DdlDiff.ALLOW_RECREATE_CONSTRAINTS_OPT;
import static com.google.cloud.solutions.spannerddl.diff.DdlDiff.ALLOW_RECREATE_INDEXES_OPT;
import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.Assert.fail;

import com.google.cloud.solutions.spannerddl.testUtils.ReadTestDatafile;
import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import org.junit.Test;

public class DdlDiffFromFilesTest {

@Test
public void compareDddTextFiles() throws IOException {
// Uses 3 files: 2 containing DDL segments to run diffs on, 1 with the expected results
// if allowRecreateIndexes and allowDropStatements are set.

LinkedHashMap<String, String> originalSegments =
ReadTestDatafile.readDdlSegmentsFromFile("originalDdl.txt");
LinkedHashMap<String, String> newSegments =
ReadTestDatafile.readDdlSegmentsFromFile("newDdl.txt");
LinkedHashMap<String, String> expectedOutputs =
ReadTestDatafile.readDdlSegmentsFromFile("expectedDdlDiff.txt");

Iterator<Map.Entry<String, String>> originalSegmentIt = originalSegments.entrySet().iterator();
Iterator<Map.Entry<String, String>> newSegmentIt = newSegments.entrySet().iterator();
Iterator<Map.Entry<String, String>> expectedOutputIt = expectedOutputs.entrySet().iterator();

String segmentName = null;
try {
while (originalSegmentIt.hasNext()) {
Map.Entry<String, String> originalSegment = originalSegmentIt.next();
segmentName = originalSegment.getKey();
Map.Entry<String, String> newSegment = newSegmentIt.next();
Map.Entry<String, String> expectedOutput = expectedOutputIt.next();

// verify segment name order for sanity.
assertWithMessage("mismatched section names in newDdl.txt")
.that(newSegment.getKey())
.isEqualTo(segmentName);
assertWithMessage("mismatched section names in expectedDdlDiff.txt")
.that(expectedOutput.getKey())
.isEqualTo(segmentName);
List<String> expectedDiff =
expectedOutput.getValue() != null
? Arrays.asList(expectedOutput.getValue().split("\n"))
: Collections.emptyList();

DdlDiff ddlDiff = DdlDiff.build(originalSegment.getValue(), newSegment.getValue());
// Run diff with allowRecreateIndexes and allowDropStatements
List<String> diff =
ddlDiff.generateDifferenceStatements(
ImmutableMap.of(
ALLOW_RECREATE_INDEXES_OPT,
true,
ALLOW_DROP_STATEMENTS_OPT,
true,
ALLOW_RECREATE_CONSTRAINTS_OPT,
true));
// check expected results.
assertWithMessage("Mismatch for section " + segmentName).that(diff).isEqualTo(expectedDiff);

// TEST PART 2: with allowDropStatements=false

// build an expectedResults without any column or table drops.
List<String> expectedDiffNoDrops =
expectedDiff.stream()
.filter(statement -> !statement.matches(".*DROP (TABLE|COLUMN).*"))
.collect(Collectors.toCollection(LinkedList::new));

// remove any drop indexes from the expectedResults if they do not have an equivalent
// CREATE statement. This is because we are allowing recreation of indexes, but not allowing
// dropping of removed indexes.
for (String statement : expectedDiff) {
if (statement.startsWith("DROP INDEX ")) {
String indexName = statement.split(" ")[2];
// see if there is a matching create statement
Pattern p = Pattern.compile("CREATE .*INDEX " + indexName + " ");
if (expectedDiffNoDrops.stream().noneMatch(s -> p.matcher(s).find())) {
expectedDiffNoDrops.remove(statement);
}
}
}

diff =
ddlDiff.generateDifferenceStatements(
ImmutableMap.of(
ALLOW_RECREATE_INDEXES_OPT,
true,
ALLOW_DROP_STATEMENTS_OPT,
false,
ALLOW_RECREATE_CONSTRAINTS_OPT,
true));
// check expected results.
assertWithMessage("Mismatch for section (noDrops)" + segmentName)
.that(diff)
.isEqualTo(expectedDiffNoDrops);
}
} catch (Throwable e) {
e.printStackTrace(System.err);
fail("Unexpected exception when processing segment " + segmentName + ": " + e);
}
}
}
Loading
Loading