Hi
New patch for this problem. It fixes all the cases that have been found
so far.
Pre-built jar file here: http://files.mkgmap.org.uk/download/196/mkgmap.jar
..Steve
Index: src/uk/me/parabola/mkgmap/osmstyle/RuleFileReader.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/uk/me/parabola/mkgmap/osmstyle/RuleFileReader.java (revision 3152)
+++ src/uk/me/parabola/mkgmap/osmstyle/RuleFileReader.java (revision )
@@ -278,9 +278,21 @@
rearrangeExpression(op.getSecond());
swapForSelectivity((BinaryOp) op);
+
+ // Rearrange ((A&B)&C) to (A&(B&C)).
+ while (op.getFirst().isType(AND)) {
+ Op aAndB = op.getFirst();
+ Op c = op.getSecond();
+ op.setFirst(aAndB.getFirst()); // A
+
+ aAndB.setFirst(aAndB.getSecond());
+ ((BinaryOp) aAndB).setSecond(c); // a-and-b is now b-and-c
+ ((BinaryOp) op).setSecond(aAndB);
+ }
+
Op op1 = op.getFirst();
Op op2 = op.getSecond();
-
+
// If the first term is an EQUALS or EXISTS then this subtree is
// already solved and we need to do no more.
if (isSolved(op1)) {
@@ -424,9 +436,11 @@
return 10;
case AND:
- case OR:
return Math.min(selectivity(op.getFirst()), selectivity(op.getSecond()));
+
+ case OR:
+ return Math.max(selectivity(op.getFirst()), selectivity(op.getSecond()));
-
+
default:
return 1000;
}
Index: test/uk/me/parabola/mkgmap/osmstyle/RuleFileReaderTest.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- test/uk/me/parabola/mkgmap/osmstyle/RuleFileReaderTest.java (revision 3152)
+++ test/uk/me/parabola/mkgmap/osmstyle/RuleFileReaderTest.java (revision )
@@ -378,6 +378,88 @@
}
/**
+ * Failure of the optimiser to promote the correct term to the front.
+ * Example from mailing list.
+ */
+ @Test
+ public void testOptimizeWithOr() {
+ String s = "highway ~ '(secondary|tertiary|unclassified|residential|minor|living_street|service)' " +
+ "& oneway=* " +
+ "& (cycleway=opposite | cycleway=opposite_lane | cycleway=opposite_track )" +
+ "[0x2 ]";
+ RuleSet rs = makeRuleSet(s);
+
+ Element el = new Way(1);
+ el.addTag("highway", "tertiary");
+ el.addTag("oneway", "1");
+ el.addTag("cycleway", "opposite_track");
+
+ GType type = getFirstType(rs, el);
+ assertNotNull(type);
+ assertEquals(2, type.getType());
+
+ el.addTag("cycleway", "fred");
+ type = getFirstType(rs, el);
+ assertNull(type);
+
+ el.addTag("cycleway", "opposite");
+ type = getFirstType(rs, el);
+ assertNotNull(type);
+
+ el.addTag("cycleway", "opposite_lane");
+ type = getFirstType(rs, el);
+ assertNotNull(type);
+
+ el.addTag("highway", "fred");
+ type = getFirstType(rs, el);
+ assertNull(type);
+ }
+
+ /**
+ * Test is a simplified version of a rule in the floodblocker style.
+ */
+ @Test
+ public void testOptimizeWithOr2() {
+ String s = "highway=*" +
+ "& tunnel!=*" +
+ "& (layer!=* | layer=0)" +
+ " [0x02]\n"
+ ;
+ RuleSet rs = makeRuleSet(s);
+ Element el = new Way(1);
+
+ el.addTag("highway", "primary");
+ GType type = getFirstType(rs, el);
+ assertNotNull(type);
+ assertEquals(2, type.getType());
+
+ el.addTag("layer", "0");
+ type = getFirstType(rs, el);
+ assertNotNull(type);
+ assertEquals(2, type.getType());
+
+ el.addTag("layer", "1");
+ type = getFirstType(rs, el);
+ assertNull(type);
+ }
+
+ @Test
+ public void testOptimizeWithOr3() throws Exception {
+ String s = "highway=* & bridge!=* & " +
+ " (mtb:scale>0 | mtb:scale='0+' | tracktype ~ 'grade[2-6]' |" +
+ " sac_scale ~ '.*(mountain|alpine)_hiking' |" +
+ " sport=via_ferrata) [0x3]";
+
+ RuleSet rs = makeRuleSet(s);
+
+ Element el = new Way(1);
+ el.addTag("highway", "primary");
+ el.addTag("mtb:scale", "0+");
+ GType type = getFirstType(rs, el);
+ assertNotNull(type);
+ }
+
+ /**
* This simply is to make sure that actions that affect their own
* conditions do not hang. There are no defined semantics for this.
*/
@@ -596,7 +678,7 @@
Way el = new Way(1);
el.addTag("highway", "primary");
- final List<GType> list = new ArrayList<GType>();
+ final List<GType> list = new ArrayList<>();
rs.resolveType(el, new TypeResult() {
public void add(Element el, GType type) {
@@ -865,16 +947,13 @@
assertNotNull(type);
}
- @Test
+ @Test(expected = SyntaxException.class)
public void testFunctionWithParameters() {
// a parameter in a function is not allowed yet
- try {
- // this should throw a SyntaxException
- makeRuleSet("A=B & length(a) > 91 [0x5]");
- assertTrue("Function with parameters are not allowed", false);
+ // this should throw a SyntaxException
+ makeRuleSet("A=B & length(a) > 91 [0x5]");
+ assertTrue("Function with parameters are not allowed", false);
- } catch (SyntaxException exp) {
- }
+ }
- }
@Test
public void testIsClosedFunction() {
@@ -1047,7 +1126,7 @@
* resolved type.
*/
private GType getFirstType(Rule rs, Element el) {
- final List<GType> types = new ArrayList<GType>();
+ final List<GType> types = new ArrayList<>();
rs.resolveType(el, new TypeResult() {
public void add(Element el, GType type) {
types.add(type);
_______________________________________________
mkgmap-dev mailing list
[email protected]
http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
