Skip to content

Commit

Permalink
Test and fix for #125
Browse files Browse the repository at this point in the history
  • Loading branch information
@tzaeschke
tzaeschke committed May 9, 2020
1 parent 8f7df00 commit 9c5bca7
Show file tree
Hide file tree
Showing 3 changed files with 250 additions and 170 deletions.
16 changes: 2 additions & 14 deletions src/org/zoodb/internal/query/ParameterDeclaration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@
*/
public final class ParameterDeclaration {

public interface Consumer {
void setValue(ParameterDeclaration param, Object value);
}

public enum DECLARATION {
/** implicit with : */
IMPLICIT,
Expand Down Expand Up @@ -89,8 +85,8 @@ public Object getValue(Object[] params) {
return params[pos];
}

public void setValue(Object[] params, Object object) {
params[pos] = object;
public int getPosition() {
return pos;
}

public static void adjustValues(List<ParameterDeclaration> decls, Object[] params) {
Expand All @@ -102,14 +98,6 @@ public static void adjustValues(List<ParameterDeclaration> decls, Object[] param
if (type != null) {
TypeConverterTools.checkAssignability(p1, type);
}
params[i] = p1;
//TODO??
//TODO??
//TODO??
//TODO??
// if (p1 instanceof ZooPC) {
// oid = ((ZooPC)p1).jdoZooGetOid();
// }
} else {
params[i] = QueryTerm.NULL;
}
Expand Down
164 changes: 93 additions & 71 deletions src/org/zoodb/jdo/impl/QueryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ private enum EXECUTION_TYPE {
private final transient PersistenceManagerImpl pm;
private transient Extent<?> ext;
private boolean isUnmodifiable = false;
private Class<?> candCls = ZooPC.class; //TODO good default?
private Class<?> candCls = ZooPC.class;
private transient ZooClassDef candClsDef = null;
private String filter = "";

Expand All @@ -105,7 +105,7 @@ private enum EXECUTION_TYPE {
private final ArrayList<QueryVariable> variables = new ArrayList<>();

private QueryTree queryTree;
private QueryExecutor queryExecutor;
private final ThreadLocal<QueryExecutor> queryExecutor = new ThreadLocal<>();
//This is used in schema auto-create mode when the persistent class has no schema defined
private boolean isDummyQuery = false;

Expand Down Expand Up @@ -303,68 +303,87 @@ public void compile() {
}

private void compileQuery() {
//compile only if it was not already compiled, unless the filter changed...
if (queryTree != null) {
return;
}

//TODO do we really need this?
String fStr = filter;
if (rangeStr != null) {
fStr = (fStr == null) ? "" : fStr;
fStr += " range " + rangeStr;
}
if (orderingStr != null) {
fStr = (fStr == null) ? "" : fStr;
fStr += " order by " + orderingStr;
}

if (fStr == null || fStr.trim().length() == 0 || isDummyQuery) {
return;
}

if (DBStatistics.isEnabled()) {
pm.getSession().statsInc(DBStatistics.STATS.QU_COMPILED);
}

executionType = determineExecutionType();

QueryParserAPI qp;
//We do this on the query before assigning values to parameter.
//Would it make sense to assign the values first and then properly parse the query???
//Probably not:
//- every parameter change would require rebuilding the tree
//- we would require an additional parser to assign the parameters
//QueryParser qp = new QueryParser(filter, candClsDef, parameters, ordering);
//QueryParserV2 qp = new QueryParserV2(filter, candClsDef, parameters, ordering);
//QueryParserV3 qp =
// new QueryParserV3(fStr, candClsDef, parameters, variables, ordering, rangeMin, rangeMax);
if (executionType == EXECUTION_TYPE.V4 || executionType == EXECUTION_TYPE.FORCED_V4 ) {
qp = new QueryParserV4(fStr, candClsDef, parameters, variables,
ordering, rangeMin, rangeMax, pm.getSession());
} else {
qp = new QueryParserV3(fStr, candClsDef, parameters, ordering, rangeMin, rangeMax);
}
queryTree = qp.parseQuery();
rangeMin = qp.getRangeMin();
rangeMax = qp.getRangeMax();
// Protect access to queryTree instance/reference
// TODO We should improve thread safety here:
// 1) What do we want? Do we need query re-compilation at all?
// - Re-setting essential parameters (filter/ordering/range/declared params/projection)
// is nice but not really useful, and allowing recompilation makes
// concurrency hard.
// Ideal: most parameters should be immutable once the query is compiled.
// - What is the point of setUnmodifiable(), spec
// 2) Create config object with ThreadLocal<range,resultClass,ignoreCache>
// Take care that it is properly initialised.
// Create a ThreadLocal config instance, reference as primary instance.
// Initialize other instances with this reference instance.
// - BLOCK ALL OTHER CHANGES once query is compiled.
// - Parse RANGE (ORDERING?) separately from main query -> avoid reset()
synchronized (this) {
//compile only if it was not already compiled, unless the filter changed...
if (queryTree != null) {
return;
}

//TODO do we really need this?
String fStr = filter;
if (rangeStr != null) {
fStr = (fStr == null) ? "" : fStr;
fStr += " range " + rangeStr;
}
if (orderingStr != null) {
fStr = (fStr == null) ? "" : fStr;
fStr += " order by " + orderingStr;
}

if (fStr == null || fStr.trim().length() == 0 || isDummyQuery) {
return;
}

if (DBStatistics.isEnabled()) {
pm.getSession().statsInc(DBStatistics.STATS.QU_COMPILED);
}

executionType = determineExecutionType();

QueryParserAPI qp;
//We do this on the query before assigning values to parameter.
//Would it make sense to assign the values first and then properly parse the query???
//Probably not:
//- every parameter change would require rebuilding the tree
//- we would require an additional parser to assign the parameters
//QueryParser qp = new QueryParser(filter, candClsDef, parameters, ordering);
//QueryParserV2 qp = new QueryParserV2(filter, candClsDef, parameters, ordering);
//QueryParserV3 qp =
// new QueryParserV3(fStr, candClsDef, parameters, variables, ordering, rangeMin, rangeMax);
if (executionType == EXECUTION_TYPE.V4 || executionType == EXECUTION_TYPE.FORCED_V4 ) {
qp = new QueryParserV4(fStr, candClsDef, parameters, variables,
ordering, rangeMin, rangeMax, pm.getSession());
} else {
qp = new QueryParserV3(fStr, candClsDef, parameters, ordering, rangeMin, rangeMax);
}
queryTree = qp.parseQuery();
rangeMin = qp.getRangeMin();
rangeMax = qp.getRangeMax();
}
}

private void resetQuery() {
//See Test_122: We need to clear this for setFilter() calls
for (int i = 0; i < parameters.size(); i++) {
ParameterDeclaration p = parameters.get(i);
if (p.getDeclaration() != DECLARATION.API) {
parameters.remove(i);
i--;
}
}
queryTree = null;
queryExecutor = null;
ordering.clear();
if (executionType == EXECUTION_TYPE.V3 || executionType == EXECUTION_TYPE.V4) {
executionType = EXECUTION_TYPE.UNDEFINED;
}
// Protect access to queryTree instance/reference
synchronized (this) {
//See Test_122: We need to clear this for setFilter() calls
for (int i = 0; i < parameters.size(); i++) {
ParameterDeclaration p = parameters.get(i);
if (p.getDeclaration() != DECLARATION.API) {
parameters.remove(i);
i--;
}
}
queryTree = null;
queryExecutor.remove();
ordering.clear();
if (executionType == EXECUTION_TYPE.V3 || executionType == EXECUTION_TYPE.V4) {
executionType = EXECUTION_TYPE.UNDEFINED;
}
}
}

@Override
Expand Down Expand Up @@ -522,8 +541,7 @@ public Object execute() {
pm.getSession().statsInc(STATS.QU_EXECUTED_TOTAL);
pm.getSession().statsInc(STATS.QU_EXECUTED_WITHOUT_INDEX);
}
createExecutor();
return queryExecutor.runWithExtent(new ExtentAdaptor(extent),
return getOrCreateExecutor().runWithExtent(new ExtentAdaptor(extent),
rangeMin, rangeMax, resultSettings, resultClass);
} finally {
pm.getSession().unlock();
Expand All @@ -535,18 +553,22 @@ public Object execute() {
return runQuery(NO_PARAMS);
}


private void createExecutor() {
if (queryExecutor == null) {
queryExecutor = new QueryExecutor(pm.getSession(), filter, candCls, candClsDef,
unique, subClasses, isDummyQuery, ordering, queryTree, parameters, variables);
private QueryExecutor getOrCreateExecutor() {
if (queryExecutor.get() == null) {
// Protect access to queryTree instance/reference
synchronized (this) {
queryExecutor.set(new QueryExecutor(pm.getSession(), filter, candCls, candClsDef,
unique, subClasses, isDummyQuery, ordering, queryTree, parameters,
variables));
}
}
return queryExecutor.get();
}

private Object runQuery(Object[] params) {
createExecutor();
QueryExecutor executor = getOrCreateExecutor();
ParameterDeclaration.adjustValues(parameters, params);
return queryExecutor.runQuery(ext, rangeMin, rangeMax, resultSettings, resultClass,
return executor.runQuery(ext, rangeMin, rangeMax, resultSettings, resultClass,
ignoreCache, params);
}

Expand Down Expand Up @@ -593,7 +615,7 @@ public Object executeWithMap(Map parameters) {
compileQuery();
Object[] params = new Object[parameters.size()];
for (ParameterDeclaration p: this.parameters) {
p.setValue(params, parameters.get(p.getName()));
params[p.getPosition()] = parameters.get(p.getName());
}
checkParamCount(parameters.size());
return runQuery(params);
Expand Down
Loading

0 comments on commit 9c5bca7

Please sign in to comment.