Skip to content

CAY-2909 Unnecessary Disjoint-By-Id Prefetch Query #641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
m-dzianishchyts wants to merge 1 commit into
base: master
Choose a base branch
from
Open

CAY-2909 Unnecessary Disjoint-By-Id Prefetch Query #641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions cayenne/src/main/java/org/apache/cayenne/access/HierarchicalObjectResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,18 @@ private boolean processDisjointByIdNode(PrefetchTreeNode node) {
ObjRelationship relationship = processorNode.getIncoming().getRelationship();

List<DbRelationship> dbRelationships = relationship.getDbRelationships();
CayennePath pathPrefix = CayennePath.EMPTY_PATH;
if (dbRelationships.size() > 1) {
CayennePath dataRowPrefix = CayennePath.EMPTY_PATH;
CayennePath qualifierPrefix = CayennePath.EMPTY_PATH;
if (relationship.isFkThroughInheritance()) {
for (int i = 0; i < dbRelationships.size() - 1; i++) {
dataRowPrefix = dataRowPrefix.dot(dbRelationships.get(i).getName());
}
} else if (dbRelationships.size() > 1) {
// we need path prefix for flattened relationships
for (int i = dbRelationships.size() - 1; i >= 1; i--) {
pathPrefix = pathPrefix.dot(dbRelationships.get(i).getReverseRelationship().getName());
dataRowPrefix = dataRowPrefix.dot(dbRelationships.get(i).getReverseRelationship().getName());
}
qualifierPrefix = dataRowPrefix;
}

List<DataRow> parentDataRows;
Expand All @@ -159,7 +165,7 @@ private boolean processDisjointByIdNode(PrefetchTreeNode node) {

int maxIdQualifierSize = context.getParentDataDomain().getMaxIdQualifierSize();
List<DbJoin> joins = getDbJoins(relationship);
Map<DbJoin, String> joinToDataRowKey = getDataRowKeys(joins, pathPrefix);
Map<DbJoin, String> joinToDataRowKey = getDataRowKeys(joins, dataRowPrefix);

List<PrefetchSelectQuery<DataRow>> queries = new ArrayList<>();
PrefetchSelectQuery<DataRow> currentQuery = null;
Expand All @@ -171,7 +177,7 @@ private boolean processDisjointByIdNode(PrefetchTreeNode node) {
if (currentQuery == null
|| (maxIdQualifierSize > 0 && qualifiersCount + joins.size() > maxIdQualifierSize)) {

createDisjointByIdPrefetchQualifier(pathPrefix, currentQuery, joins, values);
createDisjointByIdPrefetchQualifier(qualifierPrefix, currentQuery, joins, values);

currentQuery = new PrefetchSelectQuery<>(node.getPath(), relationship);
currentQuery.fetchDataRows();
Expand All @@ -192,12 +198,12 @@ private boolean processDisjointByIdNode(PrefetchTreeNode node) {
}
}
// add final part of values
createDisjointByIdPrefetchQualifier(pathPrefix, currentQuery, joins, values);
createDisjointByIdPrefetchQualifier(qualifierPrefix, currentQuery, joins, values);

PrefetchTreeNode jointSubtree = node.cloneJointSubtree();

Expression reversePath = null;
if (relationship.isSourceIndependentFromTargetChange()) {
if (relationship.isSourceIndependentFromTargetChange() && !relationship.isFkThroughInheritance()) {
reversePath = ExpressionFactory.dbPathExp(relationship.getReverseDbRelationshipPath());
}

Expand Down
12 changes: 8 additions & 4 deletions cayenne/src/main/java/org/apache/cayenne/access/PrefetchProcessorTreeBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.cayenne.ObjectId;
import org.apache.cayenne.Persistent;
import org.apache.cayenne.exp.path.CayennePath;
import org.apache.cayenne.map.ObjRelationship;
import org.apache.cayenne.query.PrefetchProcessor;
import org.apache.cayenne.query.PrefetchTreeNode;
import org.apache.cayenne.query.QueryMetadata;
Expand Down Expand Up @@ -193,10 +194,13 @@ boolean addNode(PrefetchProcessorNode node) {
node.setParentAttachmentStrategy(new NoopParentAttachmentStrategy());
} else if (node.isJointPrefetch()) {
node.setParentAttachmentStrategy(new StackLookupParentAttachmentStrategy(node));
} else if (node.getIncoming().getRelationship().isSourceIndependentFromTargetChange()) {
node.setParentAttachmentStrategy(new JoinedIdParentAttachmentStrategy(context.getGraphManager(), node));
} else {
node.setParentAttachmentStrategy(new ResultScanParentAttachmentStrategy(node));
ObjRelationship objRelationship = node.getIncoming().getRelationship();
if (objRelationship.isSourceIndependentFromTargetChange() && !objRelationship.isFkThroughInheritance()) {
node.setParentAttachmentStrategy(new JoinedIdParentAttachmentStrategy(context.getGraphManager(), node));
} else {
node.setParentAttachmentStrategy(new ResultScanParentAttachmentStrategy(node));
}
}

if (currentNode != null) {
Expand All @@ -217,4 +221,4 @@ boolean addNode(PrefetchProcessorNode node) {
PrefetchProcessorNode getParent() {
return (nodeStack.isEmpty()) ? null : nodeStack.getLast();
}
}
}
24 changes: 20 additions & 4 deletions cayenne/src/main/java/org/apache/cayenne/access/ResultScanParentAttachmentStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import org.apache.cayenne.DataRow;
import org.apache.cayenne.Persistent;
import org.apache.cayenne.exp.path.CayennePath;
import org.apache.cayenne.map.DbJoin;
import org.apache.cayenne.map.DbRelationship;
import org.apache.cayenne.map.ObjRelationship;
Expand All @@ -37,6 +38,7 @@ class ResultScanParentAttachmentStrategy implements ParentAttachmentStrategy {
private final PrefetchProcessorNode parentNode;
private final DbJoin[] joins;
private final PrefetchProcessorNode node;
private final CayennePath prefix;

// TODO: the ivar below makes this strategy STATEFUL and non-reusable. If we need a
// stateless version down the line, will need to move this to the
Expand All @@ -56,12 +58,26 @@ class ResultScanParentAttachmentStrategy implements ParentAttachmentStrategy {
ObjRelationship relationship = node.getIncoming().getRelationship();

List<DbRelationship> dbRelationships = relationship.getDbRelationships();
if (dbRelationships.size() > 1) {
if (relationship.isFkThroughInheritance()) {
CayennePath flattenedPath = CayennePath.EMPTY_PATH;
for (int i = 0; i < dbRelationships.size() - 1; i++) {
flattenedPath = flattenedPath.dot(dbRelationships.get(i).getName());
}
prefix = flattenedPath;
} else if (dbRelationships.size() > 1) {
throw new IllegalArgumentException(
"ResultScanParentAttachmentStrategy does not work for flattened relationships");
} else {
prefix = CayennePath.EMPTY_PATH;
}

joins = dbRelationships.get(0).getJoins().toArray(new DbJoin[0]);
DbRelationship dbRelationship;
if (relationship.isFkThroughInheritance()) {
dbRelationship = dbRelationships.get(dbRelationships.size() - 1);
} else {
dbRelationship = dbRelationships.get(0);
}
joins = dbRelationship.getJoins().toArray(new DbJoin[0]);
}

public void linkToParent(DataRow row, Persistent object) {
Expand Down Expand Up @@ -118,11 +134,11 @@ private void indexParents() {
if (joins.length > 1) {
List<Object> values = new ArrayList<>(joins.length);
for (DbJoin join : joins) {
values.add(row.get(join.getSourceName()));
values.add(row.get(prefix.dot(join.getSourceName()).toString()));
}
key = values;
} else {
key = row.get(joins[0].getSourceName());
key = row.get(prefix.dot(joins[0].getSourceName()).toString());
}

List<Persistent> parents = partitionByChild.computeIfAbsent(key, k -> new ArrayList<>());
Expand Down
20 changes: 20 additions & 0 deletions cayenne/src/main/java/org/apache/cayenne/map/ObjRelationship.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,26 @@ public boolean isSourceIndependentFromTargetChange() {
return isToMany() || isFlattened() || isToDependentEntity() || !isToPK();
}

/**
* Returns a boolean indicating whether the FK is accessed through an inheritance chain
* (one or more shared-PK joins followed by the actual FK-to-PK join).
*
* @since 5.0
*/
public boolean isFkThroughInheritance() {
List<DbRelationship> dbRels = getDbRelationships();
if (dbRels.size() < 2) {
return false;
}
for (int i = 0; i < dbRels.size() - 1; i++) {
DbRelationship rel = dbRels.get(i);
if (!rel.isToDependentPK() || rel.isToMany()) {
return false;
}
}
return dbRels.get(dbRels.size() - 1).isToPK();
}

/**
* Returns true if underlying DbRelationships point to dependent entity.
*/
Expand Down
2 changes: 1 addition & 1 deletion cayenne/src/main/java/org/apache/cayenne/query/FluentSelectPrefetchRouterAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public boolean startDisjointPrefetch(PrefetchTreeNode node) {
prefetchQuery.where(classDescriptor.getEntity()
.translateToRelatedEntity(queryQualifier, prefetchPath));

if (relationship.isSourceIndependentFromTargetChange()) {
if (relationship.isSourceIndependentFromTargetChange() && !relationship.isFkThroughInheritance()) {
// setup extra result columns to be able to relate result rows to the parent result objects.
prefetchQuery.addResultPath(ExpressionFactory.dbPathExp(relationship.getReverseDbRelationshipPath()));
}
Expand Down
33 changes: 33 additions & 0 deletions cayenne/src/test/java/org/apache/cayenne/access/VerticalInheritanceIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.cayenne.query.SelectById;
import org.apache.cayenne.runtime.CayenneRuntime;
import org.apache.cayenne.test.jdbc.DBHelper;
import org.apache.cayenne.unit.di.DataChannelInterceptor;
import org.apache.cayenne.test.jdbc.TableHelper;
import org.apache.cayenne.testdo.inheritance_vertical.*;
import org.apache.cayenne.unit.di.runtime.CayenneProjects;
Expand Down Expand Up @@ -62,6 +63,9 @@ public class VerticalInheritanceIT extends RuntimeCase {
@Inject
protected CayenneRuntime runtime;

@Inject
protected DataChannelInterceptor queryInterceptor;

TableHelper ivAbstractTable;

TableHelper ivConcreteTable;
Expand Down Expand Up @@ -1268,4 +1272,33 @@ public void testInsertTwoGenericVerticalInheritanceObjects() {
final List<Persistent> boys = ObjectSelect.query(Persistent.class, "GenBoy").select(context);
assertEquals(1, boys.size());
}

@Test
public void testDisjointByIdPrefetch_ToOne_FkOnChildTable_NoExtraQuery() throws SQLException {
TableHelper ivOtherTable = new TableHelper(dbHelper, "IV_OTHER");
ivOtherTable.setColumns("ID", "NAME").setColumnTypes(Types.INTEGER, Types.VARCHAR);

TableHelper ivBaseTable = new TableHelper(dbHelper, "IV_BASE");
ivBaseTable.setColumns("ID", "NAME", "TYPE").setColumnTypes(Types.INTEGER, Types.VARCHAR, Types.CHAR);

TableHelper ivImplTable = new TableHelper(dbHelper, "IV_IMPL");
ivImplTable.setColumns("ID", "ATTR1", "OTHER1_ID").setColumnTypes(Types.INTEGER, Types.VARCHAR, Types.INTEGER);

ivOtherTable.insert(1, "other1");
ivBaseTable.insert(1, "Impl 1", "I");
ivImplTable.insert(1, "attr1", 1);

List<IvImpl> result = ObjectSelect.query(IvImpl.class)
.prefetch(IvImpl.OTHER1.disjointById())
.select(context);

assertEquals(1, result.size());
IvImpl impl = result.get(0);

queryInterceptor.runWithQueriesBlocked(() -> {
IvOther other = impl.getOther1();
assertNotNull(other);
assertEquals("other1", other.getName());
});
}
}