Do not capitalize reserved words when used as column names (cloudspan…

…nerecosystem#60)

* No not capitalize reserved words when used as column names
  (note does not work in CHECK constraint and generated column expressions)
* Also add parser validation tests from a data file

.gitignore

@@ -2,3 +2,4 @@
 /*.iml
 *.jj
 target
+/.vscode/

pom.xml

@@ -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>
@@ -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>
@@ -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>
@@ -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>

src/main/java/com/google/cloud/solutions/spannerddl/diff/ASTTreeUtils.java

@@ -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(",")
@@ -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);
   }
 }

src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTforeign_key.java

@@ -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());
   }
 
@@ -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() {

src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTgeneration_clause.java

@@ -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";
   }
 }

src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTkey_part.java

@@ -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 {
 
@@ -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();
     }
   }
 }

src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTname.java

@@ -31,6 +31,6 @@ public ASTname(DdlParser p, int id) {
 
   @Override
   public String toString() {
-    return ASTTreeUtils.tokensToString(this);
+    return ASTTreeUtils.tokensToString(this, false);
   }
 }

src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTpath.java

@@ -31,6 +31,6 @@ public ASTpath(DdlParser p, int id) {
 
   @Override
   public String toString() {
-    return ASTTreeUtils.tokensToString(this);
+    return ASTTreeUtils.tokensToString(this, false);
   }
 }

src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTstored_column.java

@@ -29,6 +29,6 @@ public ASTstored_column(DdlParser p, int id) {
 
   @Override
   public String toString() {
-    return children[0].toString();
+    return ((ASTpath) children[0]).toString();
   }
 }

src/main/jjtree-sources/DdlParser.head

@@ -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)

src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffFromFilesTest.java

@@ -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);
+    }
+  }
+}