Skip to content

Commit

Permalink
Revert "propagate type arguments from expression to function return t…
Browse files Browse the repository at this point in the history 
…ype (#3124)" (#3143)

This reverts commit 97e1de8.
  • Loading branch information
@rafaelbey
rafaelbey authored Oct 1, 2024
1 parent cdd3c3b commit 278d76b
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 36 deletions.
1 change: 0 additions & 1 deletion .../main/java/org/finos/legend/engine/language/pure/compiler/toPureGraph/CompileContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ public class CompileContext
"meta::pure::metamodel::function::property",
"meta::pure::metamodel::extension",
"meta::pure::metamodel::import",
"meta::pure::metamodel::relation",
"meta::pure::functions::date",
"meta::pure::functions::string",
"meta::pure::functions::collection",
Expand Down
10 changes: 1 addition & 9 deletions .../legend/engine/language/pure/compiler/toPureGraph/PackageableElementFifthPassBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import org.finos.legend.pure.m3.coreinstance.meta.pure.mapping.AssociationImplementation;
import org.finos.legend.pure.m3.coreinstance.meta.pure.metamodel.PackageableElement;
import org.finos.legend.pure.m3.coreinstance.meta.pure.metamodel.type.FunctionType;
import org.finos.legend.pure.m3.coreinstance.meta.pure.metamodel.type.generics.GenericType;
import org.finos.legend.pure.m3.coreinstance.meta.pure.metamodel.valuespecification.ValueSpecification;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -101,14 +100,7 @@ public PackageableElement visit(Function function)
throw e;
}
FunctionType fType = ((FunctionType) targetFunc._classifierGenericType()._typeArguments().getFirst()._rawType());
GenericType lastStatementReturnType = body.getLast()._genericType();
HelperModelBuilder.checkCompatibility(this.context, lastStatementReturnType._rawType(), body.getLast()._multiplicity(), fType._returnType()._rawType(), fType._returnMultiplicity(), "Error in function '" + packageString + "'", function.body.get(function.body.size() - 1).sourceInformation);

// concrete functions do not support type arguments (ie. the diamond within Pair<String, String>) in Engine grammar
// Hence, if the raw types are compatible per above checks, we will infer the type arguments from the expression itself
// and propagate it on the function return type to ensure the Pure graph is constructed correctly
fType._returnType()._typeArguments(lastStatementReturnType._typeArguments());

HelperModelBuilder.checkCompatibility(this.context, body.getLast()._genericType()._rawType(), body.getLast()._multiplicity(), fType._returnType()._rawType(), fType._returnMultiplicity(), "Error in function '" + packageString + "'", function.body.get(function.body.size() - 1).sourceInformation);
ctx.pop();
targetFunc._expressionSequence(body);
HelperFunctionBuilder.processFunctionSuites(function, targetFunc, this.context, ctx);
Expand Down
36 changes: 10 additions & 26 deletions ...finos/legend/engine/language/pure/compiler/test/TestRelationStoreAccessorFromGrammar.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,7 @@

package org.finos.legend.engine.language.pure.compiler.test;

import java.util.List;
import java.util.stream.Collectors;
import org.eclipse.collections.api.factory.Lists;
import org.eclipse.collections.api.tuple.Pair;
import org.finos.legend.engine.language.pure.compiler.toPureGraph.PureModel;
import org.finos.legend.engine.language.pure.compiler.toPureGraph.RelationTypeHelper;
import org.finos.legend.engine.protocol.pure.v1.model.context.PureModelContextData;
import org.finos.legend.engine.protocol.pure.v1.model.relationType.Column;
import org.finos.legend.engine.shared.core.operational.errorManagement.EngineException;
import org.finos.legend.pure.m3.coreinstance.meta.pure.metamodel.function.ConcreteFunctionDefinition;
import org.finos.legend.pure.m3.coreinstance.meta.pure.metamodel.relation.RelationType;
import org.finos.legend.pure.m3.coreinstance.meta.pure.metamodel.type.FunctionType;
import org.junit.Assert;
import org.junit.Test;

Expand All @@ -35,7 +24,7 @@ public class TestRelationStoreAccessorFromGrammar extends TestCompilationFromGra
@Test
public void testCompilationOfRelationStoreAccessor()
{
Pair<PureModelContextData, PureModel> pureModelPair = test("###Relational\n" +
test("###Relational\n" +
"Database my::Store" +
"(" +
" Table myTable" +
Expand All @@ -45,15 +34,10 @@ public void testCompilationOfRelationStoreAccessor()
" )" +
")\n" +
"###Pure\n" +
"function my::func():Relation[1]" +
"function my::func():Any[*]" +
"{" +
" #>{my::Store.myTable}#->filter(c|$c.name == 'ok');" +
"}");

ConcreteFunctionDefinition<?> func = pureModelPair.getTwo().getConcreteFunctionDefinition("my::func__Relation_1_", null);
FunctionType fType = (FunctionType) func._classifierGenericType()._typeArguments().getOnly()._rawType();
List<Column> columns = RelationTypeHelper.convert((RelationType<?>) fType._returnType()._typeArguments().getAny()._rawType()).columns;
Assert.assertEquals(Lists.mutable.with("id: Integer", "name: String"), columns.stream().map(x -> x.name + ": " + x.type).collect(Collectors.toList()));
}

@Test
Expand Down Expand Up @@ -107,7 +91,7 @@ public void testCompilationWithSchema()
"\n" +
"\n" +
"###Pure\n" +
"function my::func(): Relation[1]\n" +
"function my::func(): Any[*]\n" +
"{\n" +
" #>{my::Store.mySchema.myTable}#->filter(\n" +
" c|$c.name == 'ok'\n" +
Expand All @@ -134,7 +118,7 @@ public void testCompilationWithSchemaError()
"\n" +
"\n" +
"###Pure\n" +
"function my::func(): Relation[1]\n" +
"function my::func(): Any[*]\n" +
"{\n" +
" #>{my::Store.SchemaMissing.myTable}#->filter(\n" +
" c|$c.name == 'ok'\n" +
Expand All @@ -157,15 +141,15 @@ public void testCompilationOfRelationStoreAccessorUnknownTable()
" )" +
")\n" +
"###Pure\n" +
"function my::func():Relation[1]" +
"function my::func():Any[*]" +
"{" +
" #>{my::Store.myTabe}#->filter(c|$c.name == 'ok');" +
"}");
Assert.fail();
}
catch (EngineException e)
{
Assert.assertEquals("COMPILATION error at [4:36-56]: The table myTabe can't be found in the store Store", e.toPretty());
Assert.assertEquals("COMPILATION error at [4:31-51]: The table myTabe can't be found in the store Store", e.toPretty());
}
}

Expand All @@ -184,15 +168,15 @@ public void testCompilationOfRelationStoreAccessorUnknownColumn()
" )" +
")\n" +
"###Pure\n" +
"function my::func():Relation[1]" +
"function my::func():Any[*]" +
"{" +
" #>{my::Store.myTable}#->filter(c|$c.naeme == 'ok');" +
"}");
Assert.fail();
}
catch (EngineException e)
{
Assert.assertEquals("COMPILATION error at [4:72-76]: The column 'naeme' can't be found in the relation (id:Integer, name:String)", e.toPretty());
Assert.assertEquals("COMPILATION error at [4:67-71]: The column 'naeme' can't be found in the relation (id:Integer, name:String)", e.toPretty());
}
}

Expand All @@ -212,15 +196,15 @@ public void testCompilationErrorMissingTable()
" )" +
")\n" +
"###Pure\n" +
"function my::func():Relation[1]" +
"function my::func():Any[*]" +
"{" +
" #>{my::Store}#->filter(c|$c.naeme == 'ok');" +
"}");
Assert.fail();
}
catch (EngineException e)
{
Assert.assertEquals("COMPILATION error at [4:36-49]: Error in the accessor definition. Please provide a table.", e.toPretty());
Assert.assertEquals("COMPILATION error at [4:31-44]: Error in the accessor definition. Please provide a table.", e.toPretty());
}
}

Expand Down

0 comments on commit 278d76b

Please sign in to comment.