Handle edge cases in dataquality fetch trees (#2995)

* handle edge cases 1. no model constraints 2. only model constraints * add constraint to have at least one property or constraint in validation model
finos · Aug 5, 2024 · c794c46 · c794c46
1 parent 57a0168
commit c794c46
Show file tree

Hide file tree

Showing 8 changed files with 131 additions and 9 deletions.
diff --git a/.../finos/legend/engine/language/pure/compiler/toPureGraph/DataQualityCompilerExtension.java b/.../finos/legend/engine/language/pure/compiler/toPureGraph/DataQualityCompilerExtension.java
@@ -116,6 +116,7 @@ public Iterable<? extends Processor<?>> getExtraProcessors()
                             metamodel._context(buildDataQualityExecutionContext(dataquality, compileContext))
                                     ._filter(getFilterLambda(dataquality, compileContext))
                                     ._validationTree(buildRootGraphFetchTree(dataquality.dataQualityRootGraphFetchTree, compileContext, compileContext.pureModel.getClass(dataquality.dataQualityRootGraphFetchTree._class), null, new ProcessingContext("DataQuality")));
+                            metamodel._validate(true, SourceInformationHelper.toM3SourceInformation(dataquality.sourceInformation), compileContext.getExecutionSupport());
                         }
                 )
         );

diff --git a/...gend/engine/language/pure/compiler/toPureGraph/TestDataQualityCompilationFromGrammar.java b/...gend/engine/language/pure/compiler/toPureGraph/TestDataQualityCompilationFromGrammar.java
@@ -76,6 +76,37 @@ public void testRootClassNoModelConstraints()
                 "}");
     }
 
+    @Test
+    public void testRootClassNoStructuralConstraints()
+    {
+        TestCompilationFromGrammar.TestCompilationFromGrammarTestSuite.test(COMPILATION_PREREQUISITE_CODE +
+                "###DataQualityValidation\n" +
+                "DataQualityValidation meta::dataquality::PersonDataQualityValidation\n" +
+                "{\n" +
+                "    context: fromMappingAndRuntime(meta::dataquality::dataqualitymappings, meta::dataquality::DataQualityRuntime);\n" +
+                "    filter: p:meta::dataquality::Person[1] | $p.name=='John';\n" +
+                "    validationTree: $[\n" +
+                "      meta::dataquality::Person<mustBeOfLegalAge>{\n" +
+                "      }\n" +
+                "    ]$;\n" +
+                "}");
+    }
+
+    @Test
+    public void testRootClassEmptyTree()
+    {
+        TestCompilationFromGrammar.TestCompilationFromGrammarTestSuite.test(COMPILATION_PREREQUISITE_CODE +
+                "###DataQualityValidation\n" +
+                "DataQualityValidation meta::dataquality::PersonDataQualityValidation\n" +
+                "{\n" +
+                "    context: fromMappingAndRuntime(meta::dataquality::dataqualitymappings, meta::dataquality::DataQualityRuntime);\n" +
+                "    filter: p:meta::dataquality::Person[1] | $p.name=='John';\n" +
+                "    validationTree: $[\n" +
+                "      meta::dataquality::Person{\n" +
+                "      }\n" +
+                "    ]$;\n" +
+                "}", " at [92:1-100:1]: Error in 'meta::dataquality::PersonDataQualityValidation': Execution error at (resource: lines:92c1-100c1), \"Constraint :[mustHaveAtLeastOnePropertyOrConstraint] violated in the Class DataQualityRootGraphFetchTree\"");
+    }
 
 
 

diff --git a/...lr4/org/finos/legend/engine/language/pure/grammar/from/antlr4/DataQualityParserGrammar.g4 b/...lr4/org/finos/legend/engine/language/pure/grammar/from/antlr4/DataQualityParserGrammar.g4
@@ -59,7 +59,7 @@ filter:                               FILTER COLON combinedExpression SEMI_COLON
 dqGraphDefinition:                  GRAPH_START qualifiedName(constraintList)? graphDefinition GRAPH_END
 ;
 graphDefinition:                    BRACE_OPEN
-                                        graphPaths
+                                        (graphPaths)?
                                     BRACE_CLOSE
 ;
 graphPaths:                         (graphPath | subTypeGraphPath) (COMMA (graphPath | subTypeGraphPath))*

diff --git a/...java/org/finos/legend/engine/language/dataquality/grammar/from/DataQualityTreeWalker.java b/...java/org/finos/legend/engine/language/dataquality/grammar/from/DataQualityTreeWalker.java
@@ -105,10 +105,14 @@ private DataQualityRootGraphFetchTree visitRootGraphDefinition(DataQualityParser
     {
         List<GraphFetchTree> subTrees = new ArrayList<>();
         List<SubTypeGraphFetchTree> subTypeTrees = new ArrayList<>();
-        for (DataQualityParserGrammar.GraphPathContext graphPathContext : graphDefinitionContext.graphPaths().graphPath())
+        if (Objects.nonNull(graphDefinitionContext.graphPaths()))
         {
-            subTrees.add(this.visitGraphPathContext(graphPathContext));
+            for (DataQualityParserGrammar.GraphPathContext graphPathContext : graphDefinitionContext.graphPaths().graphPath())
+            {
+                subTrees.add(this.visitGraphPathContext(graphPathContext));
+            }
         }
+
 //        for (GraphFetchTreeParserGrammar.SubTypeGraphPathContext subTypeGraphPathContext : graphDefinitionContext.graphPaths().subTypeGraphPath())
 //        {
 //            subTypeTrees.add(this.visitSubTypeGraphPathContext(subTypeGraphPathContext));
@@ -138,7 +142,7 @@ private String visitConstraintName(DataQualityParserGrammar.DqConstraintNameCont
     private PropertyGraphFetchTree visitGraphPathContext(DataQualityParserGrammar.GraphPathContext graphPathContext)
     {
         List<GraphFetchTree> subTrees = new ArrayList<>();
-        if (graphPathContext.graphDefinition() != null)
+        if (graphPathContext.graphDefinition() != null && graphPathContext.graphDefinition().graphPaths() != null)
         {
             // validationForSubTypeTrees(graphPathContext.graphDefinition());
             for (DataQualityParserGrammar.GraphPathContext subGraphPathContext : graphPathContext.graphDefinition().graphPaths().graphPath())

diff --git a/...ava/org/finos/legend/engine/language/dataquality/grammar/from/TestDataQualityParsing.java b/...ava/org/finos/legend/engine/language/dataquality/grammar/from/TestDataQualityParsing.java
@@ -137,6 +137,67 @@ public void testParserForValidGrammar()
 
     }
 
+    @Test
+    public void testEdgeScenarios()
+    {
+        // only model constraints
+        test("###DataQualityValidation\n" +
+                "DataQualityValidation meta::external::dataquality::PersonDataQualityValidation\n" +
+                "{\n" +
+                "    context: fromDataSpace(meta::external::dataquality::PersonDataSpace, 'Local_Context');\n" +
+                "    validationTree: $[\n" +
+                "     Person<ageMustBePositive>{\n" +
+                "     }\n" +
+                "    ]$;\n" +
+                "}");
+        test("###DataQualityValidation\n" +
+                "DataQualityValidation meta::external::dataquality::PersonDataQualityValidation\n" +
+                "{\n" +
+                "    context: fromDataSpace(meta::external::dataquality::PersonDataSpace, 'Local_Context');\n" +
+                "    validationTree: $[\n" +
+                "     Person{\n" +
+                "       addresses<idMustBeValid>{\n" +
+                "       }\n" +
+                "     }\n" +
+                "    ]$;\n" +
+                "}");
+        // only structural constraints
+        test("###DataQualityValidation\n" +
+                "DataQualityValidation meta::external::dataquality::PersonDataQualityValidation\n" +
+                "{\n" +
+                "    context: fromDataSpace(meta::external::dataquality::PersonDataSpace, 'Local_Context');\n" +
+                "    validationTree: $[\n" +
+                "     Person{\n" +
+                "        age\n" +
+                "     }\n" +
+                "    ]$;\n" +
+                "}");
+        test("###DataQualityValidation\n" +
+                "DataQualityValidation meta::external::dataquality::PersonDataQualityValidation\n" +
+                "{\n" +
+                "    context: fromDataSpace(meta::external::dataquality::PersonDataSpace, 'Local_Context');\n" +
+                "    validationTree: $[\n" +
+                "     Person{\n" +
+                "       addresses{\n" +
+                "         id\n" +
+                "       }\n" +
+                "     }\n" +
+                "    ]$;\n" +
+                "}");
+        // both model and structural constraints are absent
+        // note: parser is lenient to allow empty trees but fails in compilation as constraints are added in tree pure model
+        test("###DataQualityValidation\n" +
+                "DataQualityValidation meta::external::dataquality::PersonDataQualityValidation\n" +
+                "{\n" +
+                "    context: fromDataSpace(meta::external::dataquality::PersonDataSpace, 'Local_Context');\n" +
+                "    validationTree: $[\n" +
+                "     Person{\n" +
+                "     }\n" +
+                "    ]$;\n" +
+                "}");
+
+    }
+
 
 
 }
diff --git a/...e-xt-dataquality-pure-test/src/main/resources/core_dataquality_test/dataquality_test.pure b/...e-xt-dataquality-pure-test/src/main/resources/core_dataquality_test/dataquality_test.pure
@@ -13,7 +13,7 @@ import meta::pure::profiles::*;
 import meta::pure::metamodel::serialization::grammar::*;
 
 
-function <<test.Test>> meta::external::dataquality::tests::testPlanGeneration():Boolean[1]
+function <<test.Test>> meta::external::dataquality::tests::testLambdaGeneration():Boolean[1]
 { let dqRootConstraints = meta::external::dataquality::tests::Person->getAllTypeGeneralisations()->filter(x| $x->instanceOf(ElementWithConstraints))->cast(@ElementWithConstraints).constraints->filter(x| $x.name == 'addressIDGreaterThan2');
 
   let validationTree = ^DataQualityRootGraphFetchTree<Person>(
@@ -31,6 +31,25 @@ function <<test.Test>> meta::external::dataquality::tests::testPlanGeneration():
   assert($dqLambda->isNotEmpty());
 }
 
+function <<test.Test>> meta::external::dataquality::tests::testLambdaGeneration_OnlyStructuralConstraints():Boolean[1]
+{ let dqRootConstraints = meta::external::dataquality::tests::Person->getAllTypeGeneralisations()->filter(x| $x->instanceOf(ElementWithConstraints))->cast(@ElementWithConstraints).constraints->filter(x| $x.name == 'addressIDGreaterThan2');
+
+  let validationTree = ^DataQualityRootGraphFetchTree<Person>(
+    class=meta::external::dataquality::tests::Person,
+    constraints=[],
+    subTrees=meta::external::dataquality::tests::Person->hierarchicalProperties()->filter(p | $p->isPrimitiveValueProperty())->filter(p| $p.name=='age')->map(p | ^DataQualityPropertyGraphFetchTree(property=$p))
+  );
+  let filter = p:Person[1]| $p.name=='John';
+  let dataquality = ^DataQuality<Person>(validationTree=$validationTree,
+                                         context=^meta::external::dataquality::MappingAndRuntimeDataQualityExecutionContext(runtime=^meta::core::runtime::EngineRuntime(connectionStores = testDatabaseConnection(meta::relational::tests::db, [])), mapping=meta::external::dataquality::tests::dqValidationPersonMapping),
+                                         filter=$filter);
+
+  let dqLambda = meta::external::dataquality::executeDataQualityValidation($dataquality, []);
+  assert($dqLambda->isNotEmpty());
+}
+
+
+
 
 // -------------------------------------- Test Setup -------------------------------------------------------------------------//
 // relational mappings

diff --git a/...ngine-xt-dataquality-pure/src/main/resources/core_dataquality/generation/dataquality.pure b/...ngine-xt-dataquality-pure/src/main/resources/core_dataquality/generation/dataquality.pure
@@ -267,10 +267,13 @@ function meta::external::dataquality::generateFilterQuery<T>(c:Class<T>[1], f: F
 
 
 function meta::external::dataquality::generateConstraintsNegatedORQuery<T>(c:Class<T>[1], f: FunctionExpression[1], constraints: List<Constraint>[1]):FunctionExpression[1] {
-   if ($constraints.values->size() == 1,
-    | $c->generateConstraintNegatedQuery($f, $constraints.values->at(0)) ,
-    | $c->generateORNegatedQuery($f, $constraints)
-   );
+  if ($constraints.values->isEmpty(),
+       | $f ,
+       | if ($constraints.values->size() == 1,
+             | $c->generateConstraintNegatedQuery($f, $constraints.values->at(0)) ,
+             | $c->generateORNegatedQuery($f, $constraints)
+            );
+     );
 }
 
 

diff --git a/...d-engine-xt-dataquality-pure/src/main/resources/core_dataquality/metamodel/metamodel.pure b/...d-engine-xt-dataquality-pure/src/main/resources/core_dataquality/metamodel/metamodel.pure
@@ -6,6 +6,9 @@ import meta::core::runtime::*;
 
 
 Class meta::external::dataquality::DataQualityRootGraphFetchTree<T> extends RootGraphFetchTree<T>
+[
+  mustHaveAtLeastOnePropertyOrConstraint: $this.constraints->isNotEmpty() || $this.subTrees->isNotEmpty()
+]
 {
    constraints: Constraint[*];
-Original file line number
+Diff line change
@@ Expand Up / @@ -6,6 +6,9 @@ import meta::core::runtime::*; @@
     Class meta::external::dataquality::DataQualityRootGraphFetchTree<T> extends RootGraphFetchTree<T>
+    [
+      mustHaveAtLeastOnePropertyOrConstraint: $this.constraints->isNotEmpty() || $this.subTrees->isNotEmpty()
+    ]
     {
        constraints: Constraint[*];
@@ Expand Down @@