From 39127382ebaf672eaaef81a425743194f126960e Mon Sep 17 00:00:00 2001 From: Tilmann Date: Sat, 13 Jan 2018 20:50:57 +0100 Subject: [PATCH] Fixed issue #104 characters in queries --- CHANGELOG | 4 + .../zoodb/internal/query/QueryFunction.java | 44 ++++++---- .../zoodb/internal/query/QueryParserV4.java | 27 +++++- .../zoodb/internal/query/QueryTokenizer.java | 4 +- .../internal/query/TypeConverterTools.java | 15 ++++ .../jdo/Test_173_QueryIssue99_100_104.java | 82 ++++++++++++++++++- 6 files changed, 154 insertions(+), 22 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 0437c10b..39fa6c33 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,9 @@ CHANGELOG +2018-Jan-13 +=========== +- Fixed issue #104 : Characters in queries don't work as expected + 2017-Dec-05 =========== - T.Zaeschke diff --git a/src/org/zoodb/internal/query/QueryFunction.java b/src/org/zoodb/internal/query/QueryFunction.java index 3317f0c5..0da7d7d6 100644 --- a/src/org/zoodb/internal/query/QueryFunction.java +++ b/src/org/zoodb/internal/query/QueryFunction.java @@ -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; @@ -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) { @@ -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); @@ -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(); @@ -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??? @@ -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: @@ -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; diff --git a/src/org/zoodb/internal/query/QueryParserV4.java b/src/org/zoodb/internal/query/QueryParserV4.java index 0f08a094..350292a5 100644 --- a/src/org/zoodb/internal/query/QueryParserV4.java +++ b/src/org/zoodb/internal/query/QueryParserV4.java @@ -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; @@ -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) { @@ -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; @@ -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 diff --git a/src/org/zoodb/internal/query/QueryTokenizer.java b/src/org/zoodb/internal/query/QueryTokenizer.java index d979f737..1bc644c8 100644 --- a/src/org/zoodb/internal/query/QueryTokenizer.java +++ b/src/org/zoodb/internal/query/QueryTokenizer.java @@ -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; } diff --git a/src/org/zoodb/internal/query/TypeConverterTools.java b/src/org/zoodb/internal/query/TypeConverterTools.java index 18914e24..7fec5d56 100644 --- a/src/org/zoodb/internal/query/TypeConverterTools.java +++ b/src/org/zoodb/internal/query/TypeConverterTools.java @@ -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) { @@ -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()); + } + } diff --git a/tst/org/zoodb/test/jdo/Test_173_QueryIssue99_100_104.java b/tst/org/zoodb/test/jdo/Test_173_QueryIssue99_100_104.java index d760e95f..597bb482 100644 --- a/tst/org/zoodb/test/jdo/Test_173_QueryIssue99_100_104.java +++ b/tst/org/zoodb/test/jdo/Test_173_QueryIssue99_100_104.java @@ -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; @@ -122,6 +124,15 @@ public void testIssue104() { q.setFilter("_char == 'a' || _char == 'z'"); Collection rslt = (Collection) q.execute(); assertTrue(rslt.iterator().hasNext()); + + try { + q.setFilter("_char == 'ab'"); + q.execute(); + fail(); + } catch (JDOUserException e) { + //good + } + q.closeAll(); pm.currentTransaction().commit(); @@ -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 rslt = (Collection) 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 rslt2 = (Collection) 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 rslt = (Collection) 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(); }