repl: fix handling of column name that needs quotes

finos · Dec 22, 2024 · 348636c · 348636c
1 parent c3ba1be
commit 348636c
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 53 deletions.
diff --git a/...in/java/org/finos/legend/engine/language/pure/grammar/from/RelationalParseTreeWalker.java b/...in/java/org/finos/legend/engine/language/pure/grammar/from/RelationalParseTreeWalker.java
@@ -203,7 +203,7 @@ private Column visitColumnDefinition(RelationalParserGrammar.ColumnDefinitionCon
     {
         Column column = new Column();
         column.sourceInformation = this.walkerSourceInformation.getSourceInformation(ctx);
-        column.name = ctx.relationalIdentifier().getText();
+        column.name =  ctx.relationalIdentifier().getText();
         boolean nullable = true;
         if (ctx.PRIMARY_KEY() != null)
         {

diff --git a/...ava/org/finos/legend/engine/language/pure/grammar/to/HelperRelationalGrammarComposer.java b/...ava/org/finos/legend/engine/language/pure/grammar/to/HelperRelationalGrammarComposer.java
@@ -18,15 +18,7 @@
 import org.eclipse.collections.impl.utility.LazyIterate;
 import org.eclipse.collections.impl.utility.ListIterate;
 import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.mapping.PropertyMapping;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.authentication.ApiTokenAuthenticationStrategy;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.authentication.AuthenticationStrategy;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.authentication.DefaultH2AuthenticationStrategy;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.authentication.DelegatedKerberosAuthenticationStrategy;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.authentication.GCPApplicationDefaultCredentialsAuthenticationStrategy;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.authentication.GCPWorkloadIdentityFederationAuthenticationStrategy;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.authentication.MiddleTierUserNamePasswordAuthenticationStrategy;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.authentication.TestDatabaseAuthenticationStrategy;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.authentication.UserNamePasswordAuthenticationStrategy;
+import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.authentication.*;
 import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.postprocessor.Mapper;
 import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.postprocessor.MapperPostProcessor;
 import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.postprocessor.SchemaNameMapper;
@@ -35,51 +27,22 @@
 import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.specification.EmbeddedH2DatasourceSpecification;
 import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.specification.LocalH2DatasourceSpecification;
 import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.connection.specification.StaticDatasourceSpecification;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.mapping.EmbeddedRelationalPropertyMapping;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.mapping.FilterMapping;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.mapping.InlineEmbeddedPropertyMapping;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.mapping.OtherwiseEmbeddedRelationalPropertyMapping;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.mapping.RelationalPropertyMapping;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.Column;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.ColumnMapping;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.Schema;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.Table;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.TabularFunction;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.View;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.BigInt;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.Binary;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.Bit;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.Char;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.Date;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.Decimal;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.Json;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.Numeric;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.Other;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.Real;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.SemiStructured;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.SmallInt;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.Timestamp;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.TinyInt;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.VarChar;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.Varbinary;
+import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.mapping.*;
+import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.*;
+import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.datatype.*;
 import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.milestoning.BusinessMilestoning;
 import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.milestoning.BusinessSnapshotMilestoning;
 import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.milestoning.Milestoning;
 import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.milestoning.ProcessingMilestoning;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.operation.DynaFunc;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.operation.ElementWithJoins;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.operation.JoinPointer;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.operation.Literal;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.operation.LiteralList;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.operation.RelationalOperationElement;
-import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.operation.TableAliasColumn;
+import org.finos.legend.engine.protocol.pure.v1.model.packageableElement.store.relational.model.operation.*;
 import org.finos.legend.engine.shared.core.api.grammar.RenderStyle;
 
+import java.lang.Double;
+import java.lang.Float;
+import java.lang.Integer;
 import java.util.List;
 
-import static org.finos.legend.engine.language.pure.grammar.to.PureGrammarComposerUtility.convertString;
-import static org.finos.legend.engine.language.pure.grammar.to.PureGrammarComposerUtility.getTabString;
-import static org.finos.legend.engine.language.pure.grammar.to.PureGrammarComposerUtility.unsupported;
+import static org.finos.legend.engine.language.pure.grammar.to.PureGrammarComposerUtility.*;
 
 public class HelperRelationalGrammarComposer
 {
@@ -112,7 +75,7 @@ else if (op instanceof Literal)
 
     public static String renderRelationalOperationElement(RelationalOperationElement op, RelationalGrammarComposerContext context)
     {
-       return renderRelationalOperationElement(op, context, false, 0);
+        return renderRelationalOperationElement(op, context, false, 0);
     }
 
     private static String renderDynaFunc(DynaFunc dynaFunc, RelationalGrammarComposerContext context, boolean nested, int numTabs)
@@ -152,7 +115,7 @@ private static String renderDynaFunc(DynaFunc dynaFunc, RelationalGrammarCompose
                     }
                     else
                     {
-                        return  LazyIterate.collect(dynaFunc.parameters, param -> renderRelationalOperationElement(param, context)).makeString(" " + PureGrammarComposerUtility.convertIdentifier(dynaFunc.funcName) + " ");
+                        return LazyIterate.collect(dynaFunc.parameters, param -> renderRelationalOperationElement(param, context)).makeString(" " + PureGrammarComposerUtility.convertIdentifier(dynaFunc.funcName) + " ");
                     }
                 }
                 case "isNull":
@@ -263,7 +226,7 @@ private static String renderElementWithJoins(ElementWithJoins elementWithJoins,
             if (elementWithJoins.joins.size() > 1)
             {
                 builder.append(" > ");
-                String joins = LazyIterate.collect((elementWithJoins.joins.subList(1,elementWithJoins.joins.size())), HelperRelationalGrammarComposer::renderJoinPointer).makeString(" > ");
+                String joins = LazyIterate.collect((elementWithJoins.joins.subList(1, elementWithJoins.joins.size())), HelperRelationalGrammarComposer::renderJoinPointer).makeString(" > ");
                 builder.append(joins);
             }
         }
@@ -372,7 +335,11 @@ public static String renderDatabaseTabularFunction(TabularFunction tabularFuncti
     private static String renderDatabaseTableColumn(Column column, List<String> primaryKeys, int baseIndentation)
     {
         StringBuilder builder = new StringBuilder();
-        builder.append(getTabString(baseIndentation)).append(column.name).append(" ");
+        builder.append(getTabString(baseIndentation)).append(
+                // NOTE: for backward compatibility, we have to keep the current behavior of storing quotes as part of column name if present
+                // so the composer need to compensate respectively
+                column.name.startsWith("\"") && column.name.endsWith("\"") ? column.name : PureGrammarComposerUtility.convertIdentifier(column.name, true)
+        ).append(" ");
         if (column.type instanceof Char)
         {
             builder.append("CHAR(").append(((Char) column.type).size).append(")");
@@ -723,7 +690,7 @@ else if (_auth instanceof DelegatedKerberosAuthenticationStrategy)
         }
         else if (_auth instanceof MiddleTierUserNamePasswordAuthenticationStrategy)
         {
-            MiddleTierUserNamePasswordAuthenticationStrategy auth = (MiddleTierUserNamePasswordAuthenticationStrategy)_auth;
+            MiddleTierUserNamePasswordAuthenticationStrategy auth = (MiddleTierUserNamePasswordAuthenticationStrategy) _auth;
             int baseIndentation = 1;
             return "MiddleTierUserNamePassword" +
                     (auth.vaultReference != null

diff --git a/...ava/org/finos/legend/engine/language/pure/grammar/test/TestRelationalGrammarComposer.java b/...ava/org/finos/legend/engine/language/pure/grammar/test/TestRelationalGrammarComposer.java
@@ -149,4 +149,30 @@ public void TestMappingWithLeftOuterJoin() throws Exception
 
         Assert.assertEquals(expected, formatted);
     }
+
+    @Test
+    public void columnNameWithQuotes() throws Exception
+    {
+        PureModelContextData context = objectMapper.readValue(Objects.requireNonNull(getClass().getClassLoader().getResourceAsStream("columnNameWithQuotes.json")), PureModelContextData.class);
+        PureGrammarComposer grammarTransformer = PureGrammarComposer.newInstance(PureGrammarComposerContext.Builder.newInstance().build());
+        String formatted = grammarTransformer.renderPureModelContextData(context);
+
+        // Test checks that we do not output schema name before {target} - the grammar round trip tests do not exercise this code path as the graph
+        // produced for the roundtrip always sets the schema name in the table alias to default.
+        // This is testing out a graph as would be produced by tools such as Studio and checking that syntactically valid Pure is produced for self-joins
+        String expected =
+                "###Relational\n" +
+                        "Database local::DuckDuckDatabase\n" +
+                        "(\n" +
+                        "  Table sport\n" +
+                        "  (\n" +
+                        "    \"Athlete(s)\" VARCHAR(0),\n" +
+                        "    \"Age/Annee\" BIGINT,\n" +
+                        "    \"Country of Origin\" VARCHAR(0),\n" +
+                        "    \"Final Event\" VARCHAR(0)\n" +
+                        "  )\n" +
+                        ")\n";
+
+        Assert.assertEquals(expected, formatted);
+    }
 }
diff --git a/...va/org/finos/legend/engine/language/pure/grammar/test/TestRelationalGrammarRoundtrip.java b/...va/org/finos/legend/engine/language/pure/grammar/test/TestRelationalGrammarRoundtrip.java
@@ -343,6 +343,12 @@ public void testRelationalDatabase()
                 "  (\n" +
                 "    col1 CHAR(32)\n" +
                 "  )\n" +
+                "  Table table3\n" +
+                "  (\n" +
+                // handle quoted column name
+                "    \"col1\" CHAR(32),\n" +
+                "    \"this is a col\" VARCHAR(32)\n" +
+                "  )\n" +
                 "\n" +
                 "  View view1\n" +
                 "  (\n" +

diff --git a/...ion/legend-engine-xt-relationalStore-grammar/src/test/resources/columnNameWithQuotes.json b/...ion/legend-engine-xt-relationalStore-grammar/src/test/resources/columnNameWithQuotes.json
@@ -0,0 +1,60 @@
+{
+  "_type": "data",
+  "elements": [
+    {
+      "_type": "relational",
+      "filters": [],
+      "includedStores": [],
+      "joins": [],
+      "name": "DuckDuckDatabase",
+      "package": "local",
+      "schemas": [
+        {
+          "name": "default",
+          "tables": [
+            {
+              "columns": [
+                {
+                  "name": "Athlete(s)",
+                  "nullable": true,
+                  "type": {
+                    "_type": "Varchar",
+                    "size": 0
+                  }
+                },
+                {
+                  "name": "Age/Annee",
+                  "nullable": true,
+                  "type": {
+                    "_type": "BigInt"
+                  }
+                },
+                {
+                  "name": "Country of Origin",
+                  "nullable": true,
+                  "type": {
+                    "_type": "Varchar",
+                    "size": 0
+                  }
+                },
+                {
+                  "name": "\"Final Event\"",
+                  "nullable": true,
+                  "type": {
+                    "_type": "Varchar",
+                    "size": 0
+                  }
+                }
+              ],
+              "milestoning": [],
+              "name": "sport",
+              "primaryKey": []
+            }
+          ],
+          "views": []
+        }
+      ],
+      "stereotypes": []
+    }
+  ]
+}
diff --git a/pom.xml b/pom.xml
@@ -110,7 +110,7 @@
         <!-- Legend -->
         <legend.pure.version>5.28.0</legend.pure.version>
         <legend.shared.version>0.25.7</legend.shared.version>
-        <legend.web-application.version>12.72.0</legend.web-application.version>
+        <legend.web-application.version>12.83.0</legend.web-application.version>
 
         <!-- SONAR -->
         <sonar.projectKey>legend-engine</sonar.projectKey>