DRILL-5838: Fix MaprDB filter pushdown for the case of nested field (reg. of DRILL-4264)
Use FieldPath.asPathString() method instead of creating custom string representation of SchemaPath closes #972 Project: http://git-wip-us.apache.org/repos/asf/drill/repo Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/bad6c1c9 Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/bad6c1c9 Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/bad6c1c9 Branch: refs/heads/master Commit: bad6c1c991a18fe360c5ecff1704692c76438542 Parents: f781ce1 Author: Volodymyr Vysotskyi <vvo...@gmail.com> Authored: Tue Sep 26 16:37:03 2017 +0000 Committer: Paul Rogers <prog...@maprtech.com> Committed: Mon Oct 16 12:04:08 2017 -0700 ---------------------------------------------------------------------- .../mapr/db/json/JsonConditionBuilder.java | 30 +++++------ .../store/mapr/db/util/FieldPathHelper.java | 53 ++++++++++++++++++++ .../drill/maprdb/tests/json/TestSimpleJson.java | 19 ++++--- 3 files changed, 77 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/drill/blob/bad6c1c9/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/json/JsonConditionBuilder.java ---------------------------------------------------------------------- diff --git a/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/json/JsonConditionBuilder.java b/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/json/JsonConditionBuilder.java index e8a8b6e..66f2efc 100644 --- a/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/json/JsonConditionBuilder.java +++ b/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/json/JsonConditionBuilder.java @@ -20,9 +20,9 @@ package org.apache.drill.exec.store.mapr.db.json; import org.apache.drill.common.expression.BooleanOperator; import org.apache.drill.common.expression.FunctionCall; import org.apache.drill.common.expression.LogicalExpression; -import org.apache.drill.common.expression.SchemaPath; import org.apache.drill.common.expression.visitors.AbstractExprVisitor; import org.apache.drill.exec.store.hbase.DrillHBaseConstants; +import org.apache.drill.exec.store.mapr.db.util.FieldPathHelper; import org.ojai.Value; import org.ojai.store.QueryCondition; import org.ojai.store.QueryCondition.Op; @@ -159,73 +159,73 @@ public class JsonConditionBuilder extends AbstractExprVisitor<JsonScanSpec, Void private JsonScanSpec createJsonScanSpec(FunctionCall call, CompareFunctionsProcessor processor) { String functionName = processor.getFunctionName(); - SchemaPath field = processor.getPath(); + String fieldPath = FieldPathHelper.schemaPathToFieldPath(processor.getPath()).asPathString(); Value fieldValue = processor.getValue(); QueryCondition cond = null; switch (functionName) { case "equal": cond = MapRDB.newCondition(); - setIsCondition(cond, field.getRootSegmentPath(), Op.EQUAL, fieldValue); + setIsCondition(cond, fieldPath, Op.EQUAL, fieldValue); cond.build(); break; case "not_equal": cond = MapRDB.newCondition(); - setIsCondition(cond, field.getRootSegmentPath(), Op.NOT_EQUAL, fieldValue); + setIsCondition(cond, fieldPath, Op.NOT_EQUAL, fieldValue); cond.build(); break; case "less_than": cond = MapRDB.newCondition(); - setIsCondition(cond, field.getRootSegmentPath(), Op.LESS, fieldValue); + setIsCondition(cond, fieldPath, Op.LESS, fieldValue); cond.build(); break; case "less_than_or_equal_to": cond = MapRDB.newCondition(); - setIsCondition(cond, field.getRootSegmentPath(), Op.LESS_OR_EQUAL, fieldValue); + setIsCondition(cond, fieldPath, Op.LESS_OR_EQUAL, fieldValue); cond.build(); break; case "greater_than": cond = MapRDB.newCondition(); - setIsCondition(cond, field.getRootSegmentPath(), Op.GREATER, fieldValue); + setIsCondition(cond, fieldPath, Op.GREATER, fieldValue); cond.build(); break; case "greater_than_or_equal_to": cond = MapRDB.newCondition(); - setIsCondition(cond, field.getRootSegmentPath(), Op.GREATER_OR_EQUAL, fieldValue); + setIsCondition(cond, fieldPath, Op.GREATER_OR_EQUAL, fieldValue); cond.build(); break; case "isnull": - cond = MapRDB.newCondition().notExists(field.getRootSegmentPath()).build(); + cond = MapRDB.newCondition().notExists(fieldPath).build(); break; case "isnotnull": - cond = MapRDB.newCondition().exists(field.getRootSegmentPath()).build(); + cond = MapRDB.newCondition().exists(fieldPath).build(); break; case "istrue": - cond = MapRDB.newCondition().is(field.getRootSegmentPath(), Op.EQUAL, true).build(); + cond = MapRDB.newCondition().is(fieldPath, Op.EQUAL, true).build(); break; case "isnotfalse": - cond = MapRDB.newCondition().is(field.getRootSegmentPath(), Op.NOT_EQUAL, false).build(); + cond = MapRDB.newCondition().is(fieldPath, Op.NOT_EQUAL, false).build(); break; case "isfalse": - cond = MapRDB.newCondition().is(field.getRootSegmentPath(), Op.EQUAL, false).build(); + cond = MapRDB.newCondition().is(fieldPath, Op.EQUAL, false).build(); break; case "isnottrue": - cond = MapRDB.newCondition().is(field.getRootSegmentPath(), Op.NOT_EQUAL, true).build(); + cond = MapRDB.newCondition().is(fieldPath, Op.NOT_EQUAL, true).build(); break; case "like": - cond = MapRDB.newCondition().like(field.getRootSegmentPath(), fieldValue.getString()).build(); + cond = MapRDB.newCondition().like(fieldPath, fieldValue.getString()).build(); break; default: http://git-wip-us.apache.org/repos/asf/drill/blob/bad6c1c9/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/util/FieldPathHelper.java ---------------------------------------------------------------------- diff --git a/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/util/FieldPathHelper.java b/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/util/FieldPathHelper.java new file mode 100644 index 0000000..9e0621a --- /dev/null +++ b/contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/util/FieldPathHelper.java @@ -0,0 +1,53 @@ +/* +* Licensed to the Apache Software Foundation (ASF) under one or more +* contributor license agreements. See the NOTICE file distributed with +* this work for additional information regarding copyright ownership. +* The ASF licenses this file to you under the Apache License, Version 2.0 +* (the "License"); you may not use this file except in compliance with +* the License. You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ +package org.apache.drill.exec.store.mapr.db.util; + +import com.google.common.collect.Queues; +import org.apache.drill.common.expression.PathSegment; +import org.apache.drill.common.expression.SchemaPath; +import org.ojai.FieldPath; +import org.ojai.FieldSegment; +import java.util.Deque; + +public class FieldPathHelper { + + /** + * Returns {@link FieldPath} equivalent of the specified {@link SchemaPath}. + * + * @param schemaPath {@link SchemaPath} instance that should be converted + * @return {@link FieldPath} equivalent of the specified {@link SchemaPath}. + */ + public static FieldPath schemaPathToFieldPath(SchemaPath schemaPath) { + Deque<PathSegment> pathSegments = Queues.newArrayDeque(); + PathSegment pathSegment = schemaPath.getRootSegment(); + while (pathSegment != null) { + pathSegments.push(pathSegment); + pathSegment = pathSegment.getChild(); + } + + FieldSegment child = null; + while (!pathSegments.isEmpty()) { + pathSegment = pathSegments.pop(); + if (pathSegment.isNamed()) { + child = new FieldSegment.NameSegment(((PathSegment.NameSegment) pathSegment).getPath(), child, false); + } else { + child = new FieldSegment.IndexSegment(String.valueOf(((PathSegment.ArraySegment) pathSegment).getIndex()), child); + } + } + return new FieldPath((FieldSegment.NameSegment) child); + } +} http://git-wip-us.apache.org/repos/asf/drill/blob/bad6c1c9/contrib/format-maprdb/src/test/java/com/mapr/drill/maprdb/tests/json/TestSimpleJson.java ---------------------------------------------------------------------- diff --git a/contrib/format-maprdb/src/test/java/com/mapr/drill/maprdb/tests/json/TestSimpleJson.java b/contrib/format-maprdb/src/test/java/com/mapr/drill/maprdb/tests/json/TestSimpleJson.java index 0d9a991..998aae6 100644 --- a/contrib/format-maprdb/src/test/java/com/mapr/drill/maprdb/tests/json/TestSimpleJson.java +++ b/contrib/format-maprdb/src/test/java/com/mapr/drill/maprdb/tests/json/TestSimpleJson.java @@ -1,4 +1,4 @@ -/** +/* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -248,8 +248,7 @@ public class TestSimpleJson extends BaseJsonTest { + "FROM\n" + " hbase.`business` b\n" + "WHERE\n" - + " b.`attributes.Ambience.casual` = false" - ; + + " b.attributes.Ambience.casual = false"; runSQLAndVerifyCount(sql, 1); final String[] expectedPlan = {"condition=\\(attributes.Ambience.casual = false\\)"}; @@ -266,7 +265,7 @@ public class TestSimpleJson extends BaseJsonTest { + "FROM\n" + " hbase.`business` b\n" + "WHERE\n" - + " b.`attributes.Attire` = 'casual'" + + " b.attributes.Attire = 'casual'" ; runSQLAndVerifyCount(sql, 4); @@ -284,7 +283,7 @@ public class TestSimpleJson extends BaseJsonTest { + "FROM\n" + " hbase.`business` business\n" + "WHERE\n" - + " business.`attributes.Ambience.casual` IS NULL" + + " business.attributes.Ambience.casual IS NULL" ; runSQLAndVerifyCount(sql, 7); @@ -303,7 +302,7 @@ public class TestSimpleJson extends BaseJsonTest { + "FROM\n" + " hbase.`business` b\n" + "WHERE\n" - + " b.`attributes.Ambience.casual` IS NOT NULL" + + " b.attributes.Ambience.casual IS NOT NULL" ; runSQLAndVerifyCount(sql, 3); @@ -321,7 +320,7 @@ public class TestSimpleJson extends BaseJsonTest { + "FROM\n" + " hbase.`business` b\n" + "WHERE\n" - + " b.`attributes.Accepts Credit Cards` IS NULL" + + " b.attributes.`Accepts Credit Cards` IS NULL" ; runSQLAndVerifyCount(sql, 3); @@ -355,7 +354,7 @@ public class TestSimpleJson extends BaseJsonTest { + "FROM\n" + " hbase.`business` business\n" + "WHERE\n" - + " business.`attributes.Good For.lunch` = true AND" + + " business.attributes.`Good For`.lunch = true AND" + " stars > 4.1" ; runSQLAndVerifyCount(sql, 1); @@ -374,7 +373,7 @@ public class TestSimpleJson extends BaseJsonTest { + "FROM\n" + " hbase.`business` business\n" + "WHERE\n" - + " business.`hours.Tuesday.open` < TIME '10:30:00'" + + " business.hours.Tuesday.`open` < TIME '10:30:00'" ; runSQLAndVerifyCount(sql, 1); @@ -391,7 +390,7 @@ public class TestSimpleJson extends BaseJsonTest { + "FROM\n" + " hbase.`business` business\n" + "WHERE\n" - + " business.`hours.Sunday.close` > TIME '20:30:00'" + + " business.hours.Sunday.`close` > TIME '20:30:00'" ; runSQLAndVerifyCount(sql, 3);