Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
TEIID-5171 correcting forward navigation of self relationships
also refactoring so that predicate building is shared and simplified the
schema generation logic
  • Loading branch information
shawkins committed Nov 28, 2017
1 parent a8add15 commit 2a06f76
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 129 deletions.
81 changes: 39 additions & 42 deletions olingo/src/main/java/org/teiid/olingo/service/DocumentNode.java
Expand Up @@ -176,14 +176,6 @@ static Column findColumn(Table table, String propertyName) {
return table.getColumnByName(propertyName);
}

public void buildEntityKeyCriteria(UriInfo uriInfo, MetadataStore metadata, OData odata,
UniqueNameGenerator nameGenerator, URLParseService parseService) throws TeiidException {
// URL is like /entitySet(key)s
if (getKeyPredicates() != null && !getKeyPredicates().isEmpty()) {
this.criteria = buildEntityKeyCriteria(this, uriInfo, metadata, odata, nameGenerator, parseService);
}
}

public DocumentNode() {
}

Expand Down Expand Up @@ -400,30 +392,22 @@ private void addProjectedColumns(Select select, AtomicInteger ordinal,
}
}

DocumentNode joinTable(DocumentNode joinResource, EdmNavigationProperty property, JoinType joinType) throws TeiidException {
ForeignKey fk = null;
boolean reverse = false;
if (property.isCollection()) {
fk = joinFK(joinResource.getTable(), getTable(), property);
reverse = true;
}
else {
fk = joinFK(getTable(), joinResource.getTable(), property);
}

// reverse lookup
if (fk == null) {
if (property.isCollection()) {
fk = joinFK(getTable(), joinResource.getTable(), property);
}
else {
fk = joinFK(joinResource.getTable(), getTable(), property);
reverse = true;
Criteria buildJoinCriteria(DocumentNode joinResource, EdmNavigationProperty property) throws TeiidException {
KeyInfo keyInfo = joinFK(joinResource.getTable(), getTable(), property);
if (keyInfo == null) {
keyInfo = joinFK(getTable(), joinResource.getTable(), property);
if (keyInfo == null) {
throw new TeiidException("Fk not found");
}
}

if (fk == null && !joinType.equals(JoinType.JOIN_CROSS)) {
throw new TeiidException("Fk not found");
return buildCriteria(keyInfo.reverse?joinResource:this, keyInfo.reverse?this:joinResource, keyInfo.fk);
}

DocumentNode joinTable(DocumentNode joinResource, EdmNavigationProperty property, JoinType joinType) throws TeiidException {
Criteria crit = null;
if (!joinType.equals(JoinType.JOIN_CROSS)) {
crit = buildJoinCriteria(joinResource, property);
}

FromClause fromClause;
Expand All @@ -432,10 +416,6 @@ DocumentNode joinTable(DocumentNode joinResource, EdmNavigationProperty property
fromClause = new UnaryFromClause(joinResource.getGroupSymbol());
}
else {
Criteria crit = null;
if (!joinType.equals(JoinType.JOIN_CROSS)) {
crit = buildCriteria(reverse?joinResource:this, reverse?this:joinResource, fk);
}
fromClause = new JoinPredicate(this.getFromClause(), new UnaryFromClause(joinResource.getGroupSymbol()), joinType, crit);
}

Expand All @@ -450,17 +430,35 @@ static ForeignKey joinFK(DocumentNode current, DocumentNode reference, EdmNaviga
return null;
}

return joinFK(currentTable, referenceTable, property);
KeyInfo keyInfo = joinFK(currentTable, referenceTable, property);
if (keyInfo != null) {
return keyInfo.fk;
}
return null;
}

private static class KeyInfo {
boolean reverse;
ForeignKey fk;

public KeyInfo(boolean reverse, ForeignKey fk) {
this.reverse = reverse;
this.fk = fk;
}
}

private static ForeignKey joinFK(Table currentTable, Table referenceTable, EdmNavigationProperty property) {
private static KeyInfo joinFK(Table currentTable, Table referenceTable, EdmNavigationProperty property) {
for (ForeignKey fk : currentTable.getForeignKeys()) {
String refSchemaName = fk.getReferenceKey().getParent().getParent().getName();
if (((!property.isCollection() && property.getName().equals(fk.getName()))
|| (property.isCollection() && property.getName().equals(currentTable.getName() + "_" + fk.getName()))) //$NON-NLS-1$
&& referenceTable.getParent().getName().equals(refSchemaName)
&& referenceTable.getName().equals(fk.getReferenceTableName())) {
return fk;
if (!referenceTable.getParent().getName().equals(refSchemaName)
|| !referenceTable.getName().equals(fk.getReferenceTableName())) {
continue;
}
if (!property.isCollection() && property.getName().equals(fk.getName())) {
return new KeyInfo(false, fk);
}
if (property.getName().equals(currentTable.getName() + "_" + fk.getName())) { //$NON-NLS-1$
return new KeyInfo(true, fk);
}
}
return null;
Expand All @@ -477,14 +475,13 @@ static Criteria buildJoinCriteria(DocumentNode from, DocumentNode to) {

private static Criteria buildCriteria(DocumentNode from, DocumentNode to,
ForeignKey fk) {
Criteria criteria;
List<String> fkColumns = DocumentNode.getColumnNames(fk.getColumns());
if (fkColumns == null) {
fkColumns = DocumentNode.getColumnNames(getPKColumns(from.getTable()));
}

List<String> pkColumns = DocumentNode.getColumnNames(getPKColumns(to.getTable()));
criteria = DocumentNode.buildJoinCriteria(
Criteria criteria = DocumentNode.buildJoinCriteria(
from.getGroupSymbol(),
to.getGroupSymbol(), pkColumns, fkColumns);
return criteria;
Expand Down
Expand Up @@ -42,7 +42,6 @@
import org.teiid.core.types.JDBCSQLTypeInfo;
import org.teiid.core.util.Assertion;
import org.teiid.metadata.Column;
import org.teiid.metadata.ForeignKey;
import org.teiid.metadata.MetadataStore;
import org.teiid.odata.api.SQLParameter;
import org.teiid.olingo.ODataExpressionVisitor;
Expand Down Expand Up @@ -487,36 +486,8 @@ public void visit(UriResourceNavigation info) {
query.setSelect(new Select(Arrays.asList(new AggregateSymbol(AggregateSymbol.Type.COUNT.name(), false, null))));
query.setFrom(new From(Arrays.asList(navigationResource.getFromClause())));

Criteria criteria = null;
ForeignKey fk = null;
if (info.isCollection()) {
fk = DocumentNode.joinFK(navigationResource, this.ctxQuery, info.getProperty());
}
else {
fk = DocumentNode.joinFK(this.ctxQuery, navigationResource, info.getProperty());
}

if (fk != null) {
List<String> lhsColumns = DocumentNode.getColumnNames(fk.getColumns());
List<String> rhsColumns = fk.getReferenceColumns();
GroupSymbol leftSymbol = this.ctxQuery.getGroupSymbol();
GroupSymbol rightSymbol = navigationResource.getGroupSymbol();
if (info.isCollection()) {
leftSymbol = navigationResource.getGroupSymbol();
rightSymbol = this.ctxQuery.getGroupSymbol();
}
for (int i = 0; i < lhsColumns.size(); i++) {
if (criteria == null) {
criteria = new CompareCriteria(new ElementSymbol(lhsColumns.get(i), leftSymbol),
CompareCriteria.EQ, new ElementSymbol(rhsColumns.get(i), rightSymbol));
} else {
Criteria subcriteria = new CompareCriteria(new ElementSymbol(lhsColumns.get(i), leftSymbol),
CompareCriteria.EQ, new ElementSymbol(rhsColumns.get(i), rightSymbol));
criteria = new CompoundCriteria(CompoundCriteria.AND, criteria, subcriteria);
}
}
}
else {
Criteria criteria = this.ctxQuery.buildJoinCriteria(navigationResource, info.getProperty());
if (criteria == null) {
throw new TeiidException(ODataPlugin.Event.TEIID16037, ODataPlugin.Util.gs(ODataPlugin.Event.TEIID16037));
}
query.setCriteria(criteria);
Expand Down
Expand Up @@ -301,7 +301,7 @@ private void processExpandOption(ExpandOption option, DocumentNode node, Query o
}

buildAggregateQuery(node, outerQuery, expandResource,
expandOrder, query);
expandOrder, query, property);
}

if (starLevels > 0) {
Expand Down Expand Up @@ -339,18 +339,16 @@ public static void checkExpandLevel(int expandLevel)
}

private void buildAggregateQuery(DocumentNode node, Query outerQuery,
ExpandDocumentNode expandResource, OrderBy expandOrder, Query query) {
ExpandDocumentNode expandResource, OrderBy expandOrder, Query query, EdmNavigationProperty navigationProperty) throws TeiidException {
Select select = query.getSelect();
Array array = new Array(Object.class, new ArrayList<Expression>(select.getSymbols()));
select.getSymbols().clear();
AggregateSymbol symbol = new AggregateSymbol(AggregateSymbol.Type.ARRAY_AGG.name(), false, array);
select.addSymbol(symbol);
symbol.setOrderBy(expandOrder);

Criteria crit = DocumentNode.buildJoinCriteria(expandResource, node);
if (crit == null) {
crit = DocumentNode.buildJoinCriteria(node, expandResource);
}
Criteria crit = node.buildJoinCriteria(expandResource, navigationProperty);

if (crit != null) {
query.setCriteria(Criteria.combineCriteria(crit, query.getCriteria()));
} // else assertion error?
Expand Down Expand Up @@ -692,7 +690,7 @@ private void processExpand(List<ExpandNode> expand, DocumentNode resource, Query

processExpand(expandNode.children, expandResource, query, expandLevel + 1);

buildAggregateQuery(resource, outerQuery, expandResource, expandOrder, query);
buildAggregateQuery(resource, outerQuery, expandResource, expandOrder, query, expandNode.navigationProperty);
}
}

Expand Down
Expand Up @@ -209,31 +209,6 @@ static boolean isPartOfPrimaryKey(Table table, String columnName) {
return false;
}

static boolean isPrimaryKey(Table table, List<String> columnNames) {
KeyRecord pk = table.getPrimaryKey();
boolean isPK = true;
for (String columnName:columnNames) {
if (!hasColumn(pk, columnName)) {
isPK = false;
break;
}
}
if (!isPK) {
for (KeyRecord key:table.getUniqueKeys()) {
isPK = true;
for (String columnName:columnNames) {
if (!hasColumn(key, columnName)) {
isPK = false;
}
}
if (isPK) {
break;
}
}
}
return isPK;
}

static boolean hasColumn(KeyRecord pk, String columnName) {
if (pk != null) {
for (Column column : pk.getColumns()) {
Expand Down Expand Up @@ -271,10 +246,9 @@ private static void buildNavigationProperties(org.teiid.metadata.Schema schema,

// check to see if fk is part of this table's pk, then it is 1 to 1 relation
boolean fkPKSame = sameColumnSet(getIdentifier(table), fk);
boolean fkIsPK= isPrimaryKey(schema.getTable(fk.getReferenceTableName()), fk.getReferenceColumns());

addForwardNavigation(entityTypes, entitySets, table, fk, fkPKSame || fkIsPK);
addReverseNavigation(entityTypes, entitySets, table, fk, fkPKSame && fkIsPK);
addForwardNavigation(entityTypes, entitySets, table, fk, fkPKSame);
addReverseNavigation(entityTypes, entitySets, table, fk, fkPKSame);
}
}
}
Expand All @@ -289,12 +263,17 @@ private static void addForwardNavigation(Map<String, CsdlEntityType> entityTypes
navigaton = buildNavigation(fk);
navigationBinding = buildNavigationBinding(fk);

if (onetoone ) {
if (onetoone) {
navigaton.setNullable(false);
} else {
navigaton.setCollection(true);
}

for (Column c : fk.getColumns()) {
if (c.getNullType() == NullType.No_Nulls) {
navigaton.setNullable(false);
break;
}
}
}

CsdlEntityType entityType = entityTypes.get(entityTypeName);
entityType.getNavigationProperties().add(navigaton);

Expand Down
74 changes: 65 additions & 9 deletions olingo/src/test/java/org/teiid/olingo/TestODataIntegration.java
Expand Up @@ -1680,6 +1680,70 @@ public void testExpandSimple2() throws Exception {
}
}

@Test
public void testExpandComplexSelf() throws Exception {
HardCodedExecutionFactory hc = new HardCodedExecutionFactory();
hc.addData("SELECT tree.a, tree.b, tree.c FROM tree", Arrays.asList(Arrays.asList("1", "null", "x"), Arrays.asList("2", "1", "y"), Arrays.asList("3", "1", "z")));
hc.addData("SELECT tree.b, tree.a, tree.c FROM tree", Arrays.asList(Arrays.asList("null", "1", "x"), Arrays.asList("1", "2", "y"), Arrays.asList("1", "3", "z")));

teiid.addTranslator("x7", hc);

try {
ModelMetaData mmd = new ModelMetaData();
mmd.setName("m");
mmd.addSourceMetadata("ddl", "create foreign table tree ("
+ " a string, "
+ " b string, "
+ " c string, "
+ " primary key (a),"
+ " CONSTRAINT parent FOREIGN KEY (b) REFERENCES tree(a)"
+ ");");

mmd.addSourceMapping("x7", "x7", null);
teiid.deployVDB("northwind", mmd);

localClient = getClient(teiid.getDriver(), "northwind", 1, new Properties());

ContentResponse response = null;
response = http.newRequest(baseURL + "/northwind/m/tree?$expand=tree_parent($filter=$it/c%20eq%20%27x%27)")
.method("GET")
.send();

assertEquals(response.getContentAsString(), 200, response.getStatus());

assertEquals("{\"@odata.context\":\"$metadata#tree\",\"value\":["
+ "{\"a\":\"1\",\"b\":\"null\",\"c\":\"x\",\"tree_parent\":[{\"a\":\"2\",\"b\":\"1\",\"c\":\"y\"},{\"a\":\"3\",\"b\":\"1\",\"c\":\"z\"}]},"
+ "{\"a\":\"2\",\"b\":\"1\",\"c\":\"y\",\"tree_parent\":[]},"
+ "{\"a\":\"3\",\"b\":\"1\",\"c\":\"z\",\"tree_parent\":[]}]}",
response.getContentAsString());

response = http.newRequest(baseURL + "/northwind/m/tree?$expand=parent($filter=$it/c%20eq%20%27x%27)")
.method("GET")
.send();

assertEquals(response.getContentAsString(), 200, response.getStatus());

assertEquals("{\"@odata.context\":\"$metadata#tree\",\"value\":[{\"a\":\"1\",\"b\":\"null\",\"c\":\"x\",\"parent\":null},"
+ "{\"a\":\"2\",\"b\":\"1\",\"c\":\"y\",\"parent\":null},"
+ "{\"a\":\"3\",\"b\":\"1\",\"c\":\"z\",\"parent\":null}]}",
response.getContentAsString());

response = http.newRequest(baseURL + "/northwind/m/tree?$expand=parent($filter=$it/c%20eq%20%27y%27)")
.method("GET")
.send();

assertEquals(response.getContentAsString(), 200, response.getStatus());

assertEquals("{\"@odata.context\":\"$metadata#tree\",\"value\":[{\"a\":\"1\",\"b\":\"null\",\"c\":\"x\",\"parent\":null},"
+ "{\"a\":\"2\",\"b\":\"1\",\"c\":\"y\",\"parent\":{\"a\":\"1\",\"b\":\"null\",\"c\":\"x\"}},"
+ "{\"a\":\"3\",\"b\":\"1\",\"c\":\"z\",\"parent\":null}]}",
response.getContentAsString());
} finally {
localClient = null;
teiid.undeployVDB("northwind");
}
}

@Test
public void testExpandComplex() throws Exception {
HardCodedExecutionFactory hc = new HardCodedExecutionFactory();
Expand All @@ -1688,8 +1752,6 @@ public void testExpandComplex() throws Exception {
hc.addData("SELECT y.a, y.b FROM y", Arrays.asList(Arrays.asList("y", "a"), Arrays.asList("y1","a")));
hc.addData("SELECT z.a, z.b FROM z", Arrays.asList(Arrays.asList("a", "y")));
hc.addData("SELECT z.b, z.a FROM z", Arrays.asList(Arrays.asList("y", "a")));
hc.addData("SELECT tree.a, tree.b FROM tree", Arrays.asList(Arrays.asList("1", "2"), Arrays.asList("2", "3"), Arrays.asList("3", "1")));
hc.addData("SELECT tree.b, tree.a FROM tree", Arrays.asList(Arrays.asList("2", "1"), Arrays.asList("3", "2"), Arrays.asList("1", "3")));

teiid.addTranslator("x7", hc);

Expand All @@ -1713,13 +1775,7 @@ public void testExpandComplex() throws Exception {
+ " primary key (a),"
+ " CONSTRAINT FKX FOREIGN KEY (a) REFERENCES x(a),"
+ " CONSTRAINT FKY FOREIGN KEY (b) REFERENCES y(a)"
+ ") options (updatable true);"
+ "create foreign table tree ("
+ " a string, "
+ " b string, "
+ " primary key (a),"
+ " CONSTRAINT parent FOREIGN KEY (b) REFERENCES tree(a)"
+ ");");
+ ") options (updatable true);");

mmd.addSourceMapping("x7", "x7", null);
teiid.deployVDB("northwind", mmd);
Expand Down

0 comments on commit 2a06f76

Please sign in to comment.