Skip to content

Commit

Permalink
Fixed issue #104 characters in queries
Browse files Browse the repository at this point in the history
  • Loading branch information
tzaeschke committed Jan 13, 2018
1 parent ad6a1c9 commit 3912738
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 22 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
CHANGELOG

2018-Jan-13
===========
- Fixed issue #104 : Characters in queries don't work as expected

2017-Dec-05
===========
- T.Zaeschke
Expand Down
44 changes: 28 additions & 16 deletions src/org/zoodb/internal/query/QueryFunction.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static org.zoodb.internal.query.TypeConverterTools.toFloat;
import static org.zoodb.internal.query.TypeConverterTools.toInt;
import static org.zoodb.internal.query.TypeConverterTools.toLong;
import static org.zoodb.internal.query.TypeConverterTools.convertToString;

import java.lang.reflect.Field;
import java.math.BigDecimal;
Expand Down Expand Up @@ -362,22 +363,22 @@ private Object evaluateFunction(Object li, Object gi, VariableInstance[] vars) {
case MAP_containsValue: return ((Map<?,?>)li).containsValue(arg0);
case MAP_isEmpty: return ((Map<?,?>)li).isEmpty();
case MAP_size: return ((Map<?,?>)li).size();
case STR_startsWith: return ((String)li).startsWith((String) arg0);
case STR_endsWith: return ((String)li).endsWith((String) arg0);
case STR_matches: return ((String)li).matches((String) arg0);
case STR_contains_NON_JDO: return ((String)li).contains((String) arg0);
case STR_indexOf1: return ((String)li).indexOf((String) arg0);
case STR_indexOf2: return ((String)li).indexOf(
(String) arg0,
case STR_startsWith: return convertToString(li).startsWith(convertToString(arg0));
case STR_endsWith: return convertToString(li).endsWith(convertToString(arg0));
case STR_matches: return convertToString(li).matches(convertToString(arg0));
case STR_contains_NON_JDO: return convertToString(li).contains(convertToString(arg0));
case STR_indexOf1: return convertToString(li).indexOf(convertToString(arg0));
case STR_indexOf2: return convertToString(li).indexOf(
convertToString(arg0),
(Integer) arg1);
case STR_substring1: return ((String)li).substring((Integer) arg0);
case STR_substring2: return ((String)li).substring(
case STR_substring1: return convertToString(li).substring((Integer) arg0);
case STR_substring2: return convertToString(li).substring(
(Integer) arg0,
(Integer) arg1);
case STR_toLowerCase: return ((String)li).toLowerCase();
case STR_toUpperCase: return ((String)li).toUpperCase();
case STR_length: return ((String)li).length();
case STR_trim: return ((String)li).trim();
case STR_toLowerCase: return convertToString(li).toLowerCase();
case STR_toUpperCase: return convertToString(li).toUpperCase();
case STR_length: return convertToString(li).length();
case STR_trim: return convertToString(li).trim();
case Math_abs: {
Class<?> oType = arg0.getClass();
if (oType == Integer.class) {
Expand Down Expand Up @@ -420,7 +421,7 @@ private Object evaluateFunction(Object li, Object gi, VariableInstance[] vars) {
case LE:
return compare(arg0, arg1) <= 0;
case PLUS_STR:
return ((String)arg0) + ((String)arg1);
return convertToString(arg0) + convertToString(arg1);
case PLUS_D:
double d1 = TypeConverterTools.toDouble(arg0);
double d2 = TypeConverterTools.toDouble(arg1);
Expand Down Expand Up @@ -589,6 +590,10 @@ private boolean equalsObject(Object o1, Object o2) {
if (o1 instanceof Number && o2 instanceof Number) {
return compare(o1, o2) == 0;
}
if (o1 instanceof String ^ o2 instanceof String) {
// only one side is a String, try to convert the other side to String, too.
return compare(o1, o2) == 0;
}
if (o1 instanceof ZooPC && o2 instanceof ZooPC) {
long oid1 = ((ZooPC)o1).jdoZooGetOid();
long oid2 = ((ZooPC)o2).jdoZooGetOid();
Expand Down Expand Up @@ -616,7 +621,9 @@ private int compare(COMPARISON_TYPE ct, Object n1, Object n2) {
case FLOAT:
return Float.compare(toFloat(n1), toFloat(n2));
case STRING:
return ((String)n1).compareTo((String)n2);
return convertToString(n1).compareTo(convertToString(n2));
case CHAR:
return Character.compare((Character)n1, (Character) n2);
case PC:

case SCO: //TODO comparable SCO???
Expand Down Expand Up @@ -911,7 +918,11 @@ public boolean determineIndexToUseSubForQueryFunctions(
break;
case FLOAT: value = BitTools.toSortableLong(TypeConverterTools.toFloat(termVal));
break;
case CHAR: value = (long)((Character)termVal).charValue(); break;
case CHAR:
if (termVal instanceof String) {
throw DBLogger.newUser("Cannot compare String to Character");
}
value = (long)((Character)termVal).charValue(); break;
case BYTE:
case INT:
case LONG:
Expand All @@ -921,6 +932,7 @@ public boolean determineIndexToUseSubForQueryFunctions(
}
break;
case STRING:
termVal = termVal instanceof Character ? String.valueOf((Character) termVal) : termVal;
value = BitTools.toSortableLong(
termVal == QueryTerm.NULL ? null : (String)termVal);
break;
Expand Down
27 changes: 25 additions & 2 deletions src/org/zoodb/internal/query/QueryParserV4.java
Original file line number Diff line number Diff line change
Expand Up @@ -547,12 +547,23 @@ private COMPARISON_TYPE getComparisonType(QueryFunction lhs, QueryFunction rhs,
rhs.setReturnType(lrt);
rrt = lrt;
}

COMPARISON_TYPE ct = TypeConverterTools.fromTypes(lrt, rrt);
verifyOperandApplicability(tOp.type, ct, lrt, rrt);
if (ct == COMPARISON_TYPE.STRING) {
//Ensure that we are dealing with Sting, not with Character
convertCharToString(lhs);
convertCharToString(rhs);
}
return ct;
}

private void convertCharToString(QueryFunction fn) {
if (!String.class.isAssignableFrom(fn.getReturnType()) && !fn.isConstant()) {
throw tokenParsingError("Cannot compare String to Character.");
}
}

private void verifyOperandApplicability(T_TYPE op, COMPARISON_TYPE ct, Class<?> lhsCt, Class<?> rhsCt) {
if (op == T_TYPE.EQ || op == T_TYPE.NE) {
return;
Expand All @@ -567,6 +578,12 @@ private void verifyOperandApplicability(T_TYPE op, COMPARISON_TYPE ct, Class<?>
}
failOp(lhsCt, rhsCt, op);
}
if (ct == COMPARISON_TYPE.CHAR) {
if (op == T_TYPE.G || op == T_TYPE.GE || op == T_TYPE.L || op == T_TYPE.LE || op == T_TYPE.PLUS) {
return;
}
failOp(lhsCt, rhsCt, op);
}
}

private void failOp(Class<?> lhsCt, Class<?> rhsCt, T_TYPE op) {
Expand Down Expand Up @@ -601,6 +618,12 @@ private QueryFunction parseFunction(QueryFunction baseObjectFn, boolean negate)
QueryFunction cf = QueryFunction.createConstant(constant);
return tryParsingChainedFunctions(cf, negate);
}
if (match(T_TYPE.CHAR)) {
Object constant = token().str.charAt(0);
tInc();
QueryFunction cf = QueryFunction.createConstant(constant);
return tryParsingChainedFunctions(cf, negate);
}


ZooFieldDef fieldDef = null;
Expand Down Expand Up @@ -970,7 +993,7 @@ enum T_TYPE {
PARAMETERS(true), VARIABLES(true), IMPORTS(true), GROUP(true),
ORDER(true), BY(true), RANGE(true),
MATH("Math"),
THIS("this"), F_NAME, STRING, TRUE("true"), FALSE("false"), NULL("null"),
THIS("this"), F_NAME, STRING, CHAR, TRUE("true"), FALSE("false"), NULL("null"),
NUMBER_INT, NUMBER_LONG, NUMBER_FLOAT, NUMBER_DOUBLE,
LETTERS; //fieldName, paramName, JDOQL keywords

Expand Down
4 changes: 3 additions & 1 deletion src/org/zoodb/internal/query/QueryTokenizer.java
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,9 @@ private Token parseString() {
c = charAt0();
}
String value = substring(pos0, pos());
Token t = new Token(T_TYPE.STRING, value, pos0);
Token t = value.length() == 1 ?
new Token(T_TYPE.CHAR, value, pos0) :
new Token(T_TYPE.STRING, value, pos0);
inc();
return t;
}
Expand Down
15 changes: 15 additions & 0 deletions src/org/zoodb/internal/query/TypeConverterTools.java
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,15 @@ public static COMPARISON_TYPE fromOperands(COMPARISON_TYPE lhsCt,
case SHORT:
case BYTE:
case CHAR:
if (rhsCt == CHAR) {
return CHAR;
}
if (rhsCt.canBeNumber()) {
return INT;
}
if (rhsCt == STRING) {
return STRING;
}
failComp(lhsCt, rhsCt);
case BOOLEAN:
if (rhsCt == BOOLEAN) {
Expand Down Expand Up @@ -275,5 +281,14 @@ public static int toInt(Object o) {
throw DBLogger.newUser("Cannot cast type to number: " + o.getClass().getName());
}

public static String convertToString(Object o) {
if (o instanceof String) {
return (String)o;
} else if (o instanceof Character) {
return String.valueOf((Character)o);
}
throw DBLogger.newUser("Cannot cast type to String: " + o.getClass().getName());
}


}
82 changes: 79 additions & 3 deletions tst/org/zoodb/test/jdo/Test_173_QueryIssue99_100_104.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package org.zoodb.test.jdo;

import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.util.Collection;

import javax.jdo.JDOUserException;
import javax.jdo.PersistenceManager;
import javax.jdo.Query;

Expand Down Expand Up @@ -122,6 +124,15 @@ public void testIssue104() {
q.setFilter("_char == 'a' || _char == 'z'");
Collection<TestClass> rslt = (Collection<TestClass>) q.execute();
assertTrue(rslt.iterator().hasNext());

try {
q.setFilter("_char == 'ab'");
q.execute();
fail();
} catch (JDOUserException e) {
//good
}

q.closeAll();
pm.currentTransaction().commit();

Expand All @@ -147,18 +158,83 @@ public void testIssue104b() {
}
pm.currentTransaction().commit();

//test without index
pm.currentTransaction().begin();
Query q = pm.newQuery(TestClass.class);
q.setFilter("_string == 'a' || _string == 'z'");
Collection<TestClass> rslt = (Collection<TestClass>) q.execute();
assertTrue(rslt.iterator().hasNext());
q.closeAll();
pm.currentTransaction().commit();


pm.currentTransaction().begin();
ZooJdoHelper.createIndex(pm, TestClass.class, "_string", true/*isUnique*/);
pm.currentTransaction().commit();

//test with index
pm.currentTransaction().begin();
Query q2 = pm.newQuery(TestClass.class);
q2.setFilter("_string == 'a' || _string == 'z'");
Collection<TestClass> rslt2 = (Collection<TestClass>) q2.execute();
assertTrue(rslt2.iterator().hasNext());
q2.closeAll();
pm.currentTransaction().commit();

TestTools.closePM();
}

/**
* Check that comparing char fields to a String fails.
*/
@SuppressWarnings("unchecked")
@Test
public void testIssue104c() {
ZooJdoProperties props = TestTools.getProps();
props.setZooAutoCreateSchema(true);
PersistenceManager pm = TestTools.openPM(props);

final String str = "abcdefghijklmnopqrstuvwxyz";
pm.currentTransaction().begin();
for (char c : str.toCharArray()) {
TestClass i = new TestClass();
i.setChar(c);
pm.makePersistent(i);
}
pm.currentTransaction().commit();


//Test without index
pm.currentTransaction().begin();
Query q = pm.newQuery(TestClass.class);
q.setFilter("_string == 'a' || _string == 'z'");
Collection<TestClass> rslt = (Collection<TestClass>) q.execute();
assertTrue(rslt.iterator().hasNext());
try {
q.setFilter("_char == 'ab'");
q.execute();
fail();
} catch (JDOUserException e) {
//good
}
q.closeAll();
pm.currentTransaction().commit();


pm.currentTransaction().begin();
ZooJdoHelper.createIndex(pm, TestClass.class, "_char", true/*isUnique*/);
pm.currentTransaction().commit();

//test with index
pm.currentTransaction().begin();
Query q2 = pm.newQuery(TestClass.class);
try {
q2.setFilter("_char == 'ab'");
q2.execute();
fail();
} catch (JDOUserException e) {
//good
}
q2.closeAll();
pm.currentTransaction().commit();

TestTools.closePM();
}

Expand Down

0 comments on commit 3912738

Please sign in to comment.