Skip to content
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.

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

[WIP] Optimized the DependantTable handling #3150

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@ public final class OrmQueryRequest<T> extends BeanRequest implements SpiOrmQuery
private PersistenceContext persistenceContext;
private HashQuery cacheKey;
private CQueryPlanKey queryPlanKey;
// The queryPlan during the request.
private CQueryPlan queryPlan;
private SpiQuerySecondary secondaryQueries;
private List<T> cacheBeans;
private BeanPropertyAssocMany<?> manyProperty;
private boolean inlineCountDistinct;
private Set<String> dependentTables;
private boolean prepared;

public OrmQueryRequest(SpiEbeanServer server, OrmQueryEngine queryEngine, SpiQuery<T> query, SpiTransaction t) {
super(server, t);
Expand All @@ -68,6 +70,7 @@ public boolean isDeleteByStatement() {
} else {
// delete by ids due to cascading delete needs
queryPlanKey = query.setDeleteByIdsPlan();
queryPlan = null;
return false;
}
}
Expand Down Expand Up @@ -172,10 +175,24 @@ private void adapterPreQuery() {
*/
@Override
public void prepareQuery() {
secondaryQueries = query.convertJoins();
beanDescriptor.prepareQuery(query);
adapterPreQuery();
queryPlanKey = query.prepare(this);
if (!prepared) {
secondaryQueries = query.convertJoins();
beanDescriptor.prepareQuery(query);
adapterPreQuery();
queryPlanKey = query.prepare(this);
prepared = true;
}

}

/**
* The queryPlanKey has to be updated, if elements are removed from an already prepared query.
*/
private void updateQueryPlanKey() {
if (prepared) {
queryPlanKey = query.prepare(this);
queryPlan = null;
}
}

public boolean isNativeSql() {
Expand Down Expand Up @@ -470,7 +487,10 @@ public BeanPropertyAssocMany<?> manyPropertyForOrderBy() {
* query plan for this query exists.
*/
public CQueryPlan queryPlan() {
return beanDescriptor.queryPlan(queryPlanKey);
if (queryPlan == null) {
queryPlan = beanDescriptor.queryPlan(queryPlanKey);
}
return queryPlan;
}

/**
Expand All @@ -488,6 +508,7 @@ public CQueryPlanKey queryPlanKey() {
* Put the QueryPlan into the cache.
*/
public void putQueryPlan(CQueryPlan queryPlan) {
this.queryPlan = queryPlan;
beanDescriptor.queryPlan(queryPlanKey, queryPlan);
}

Expand All @@ -497,7 +518,7 @@ public void resetBeanCacheAutoMode(boolean findOne) {
}

public boolean isQueryCachePut() {
return cacheKey != null && query.queryCacheMode().isPut();
return cacheKey != null && queryPlan != null && query.queryCacheMode().isPut();
}

public boolean isBeanCachePutMany() {
Expand Down Expand Up @@ -606,7 +627,14 @@ public boolean getFromBeanCache() {
BeanCacheResult<T> cacheResult = beanDescriptor.cacheIdLookup(persistenceContext, idLookup.idValues());
// adjust the query (IN clause) based on the cache hits
this.cacheBeans = idLookup.removeHits(cacheResult);
return idLookup.allHits();
if (idLookup.allHits()) {
return true;
} else {
if (!this.cacheBeans.isEmpty()) {
updateQueryPlanKey();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHECKME: Do we have a test for this code path...

}
return false;
}
}
if (!beanDescriptor.isNaturalKeyCaching()) {
return false;
Expand All @@ -619,7 +647,14 @@ public boolean getFromBeanCache() {
BeanCacheResult<T> cacheResult = beanDescriptor.naturalKeyLookup(persistenceContext, naturalKeySet.keys());
// adjust the query (IN clause) based on the cache hits
this.cacheBeans = data.removeHits(cacheResult);
return data.allHits();
if (data.allHits()) {
return true;
} else {
if (!this.cacheBeans.isEmpty()) {
updateQueryPlanKey();
}
return false;
}
}
}
return false;
Expand Down Expand Up @@ -688,7 +723,7 @@ private boolean readAuditQueryType() {
}

public void putToQueryCache(Object result) {
beanDescriptor.queryCachePut(cacheKey, new QueryCacheEntry(result, dependentTables, transaction.startNanoTime()));
beanDescriptor.queryCachePut(cacheKey, new QueryCacheEntry(result, queryPlan.dependentTables(), transaction.startNanoTime()));
}

/**
Expand Down Expand Up @@ -758,15 +793,6 @@ public boolean isInlineCountDistinct() {
return inlineCountDistinct;
}

public void addDependentTables(Set<String> tables) {
if (tables != null && !tables.isEmpty()) {
if (dependentTables == null) {
dependentTables = new LinkedHashSet<>();
}
dependentTables.addAll(tables);
}
}

/**
* Return true if no MaxRows or use LIMIT in SQL update.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,4 @@ public void handleLoadError(String fullName, Exception e) {
query.handleLoadError(fullName, e);
}

public Set<String> dependentTables() {
return queryPlan.dependentTables();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ private <A extends Collection<?>> A findAttributeCollection(OrmQueryRequest<?> r
request.transaction().logSummary(rcQuery.summary());
}
if (request.isQueryCachePut()) {
request.addDependentTables(rcQuery.dependentTables());
if (collection instanceof List) {
collection = (A) Collections.unmodifiableList((List<?>) collection);
request.putToQueryCache(collection);
Expand Down Expand Up @@ -167,7 +166,6 @@ public <T> int findCount(OrmQueryRequest<T> request) {
request.transaction().end();
}
if (request.isQueryCachePut()) {
request.addDependentTables(rcQuery.dependentTables());
request.putToQueryCache(count);
}
return count;
Expand Down Expand Up @@ -355,9 +353,6 @@ <T> BeanCollection<T> findMany(OrmQueryRequest<T> request) {
cquery.auditFindMany();
}
request.executeSecondaryQueries(false);
if (request.isQueryCachePut()) {
request.addDependentTables(cquery.dependentTables());
}
return beanCollection;

} catch (SQLException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,6 @@ public void profile() {
.addQueryEvent(query.profileEventId(), profileOffset, desc.name(), rowCount, query.profileId());
}

Set<String> dependentTables() {
return queryPlan.dependentTables();
}

@Override
public void cancel() {
lock.lock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,6 @@ public void profile() {
.addQueryEvent(query.profileEventId(), profileOffset, desc.name(), rowCount, query.profileId());
}

Set<String> dependentTables() {
return queryPlan.dependentTables();
}

@Override
public void cancel() {
lock.lock();
Expand Down