DRILL-6865: Query returns wrong result when filter pruning happens #1552

vvysotskyi · 2018-11-23T12:43:23Z

This PR contains two commits:

The first commit contains changes to preserve predicates from filter condition which weren't used for filter pushdown to avoid the case when the filter is pruned. Instead of pruning whole the filter, only predicates which were used in the row group filtering are removed. Please note, that this problem happened only for the case when row group fully matches to the predicates which are used in the filter pushdown.
The second commit contains changes to remove filter from the plan when parquet table has a single row group and fully matches the filter.

For problem descriptions please see DRILL-6865.

arina-ielchiieva · 2018-11-23T14:04:17Z

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetPushDownFilter.java

+      if (groupScan.isMatchAllRowGroups()) {
+        RelNode child = project == null ? scan : project;
+        // if current row group fully matches filter,
+        // but row group pruning wasn't happened, removes filter.


did not happen, remove the filter

thanks, changed.

arina-ielchiieva · 2018-11-23T14:06:37Z

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/AbstractParquetGroupScan.java

@@ -85,6 +85,8 @@

  private List<EndpointAffinity> endpointAffinities;
  private ParquetGroupScanStatistics parquetGroupScanStatistics;
+  // whether all row groups of this group scan fully matches the filter


fully match

Thanks, fixed

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetPushDownFilter.java

arina-ielchiieva · 2018-11-23T14:34:08Z

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetPushDownFilter.java

+        // but row group pruning wasn't happened, removes filter.
+        if (nonConvertedPredList.size() == 0) {
+          call.transformTo(child);
+        } else if (nonConvertedPredList.size() < predList.size()) {


Won't be else enough? Why check that non converted list is smaller?

For the case when nonConvertedPredList.size() == predList.size(), none of the predicates participated in filter pushdown, so call.transformTo() shouldn't be called for this case.

This clear, my concern was second condition } else if (nonConvertedPredList.size() < predList.size()). Why we cannot use else instead?

In this case, else will include both nonConvertedPredList.size() < predList.size() and nonConvertedPredList.size() == predList.size() cases, but as I pointed in the comment above, we shouldn't do anything for the last case.

arina-ielchiieva · 2018-11-23T14:43:28Z

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFilterBuilder.java

@@ -63,6 +63,7 @@
  static final Logger logger = LoggerFactory.getLogger(ParquetFilterBuilder.class);

  private final UdfUtilities udfUtilities;
+  private final boolean omitUnsupportedExprs;


Please add javadoc explaining cases when we want to omit unsupported expressions and when we don't.

Thanks, added.

Please describe cases when we need this flag.

Added to its Javadoc case when it should be used.

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/AbstractParquetGroupScan.java

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFilterBuilder.java

vvysotskyi

@arina-ielchiieva, thanks for the review, I have addressed your comments, could you please take a look again?

vvysotskyi · 2018-11-23T17:05:23Z

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/AbstractParquetGroupScan.java

@@ -85,6 +85,8 @@

  private List<EndpointAffinity> endpointAffinities;
  private ParquetGroupScanStatistics parquetGroupScanStatistics;
+  // whether all row groups of this group scan fully matches the filter


Thanks, fixed

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/AbstractParquetGroupScan.java

vvysotskyi · 2018-11-23T17:19:20Z

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFilterBuilder.java

@@ -63,6 +63,7 @@
  static final Logger logger = LoggerFactory.getLogger(ParquetFilterBuilder.class);

  private final UdfUtilities udfUtilities;
+  private final boolean omitUnsupportedExprs;


Thanks, added.

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetPushDownFilter.java

vvysotskyi · 2018-11-23T17:27:24Z

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetPushDownFilter.java

+      if (groupScan.isMatchAllRowGroups()) {
+        RelNode child = project == null ? scan : project;
+        // if current row group fully matches filter,
+        // but row group pruning wasn't happened, removes filter.


thanks, changed.

vvysotskyi · 2018-11-23T17:30:07Z

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetPushDownFilter.java

+        // but row group pruning wasn't happened, removes filter.
+        if (nonConvertedPredList.size() == 0) {
+          call.transformTo(child);
+        } else if (nonConvertedPredList.size() < predList.size()) {


For the case when nonConvertedPredList.size() == predList.size(), none of the predicates participated in filter pushdown, so call.transformTo() shouldn't be called for this case.

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFilterBuilder.java

vvysotskyi

@arina-ielchiieva, I have made required changes, could you please take a look?

vvysotskyi · 2018-11-25T11:28:20Z

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetPushDownFilter.java

+        // but row group pruning wasn't happened, removes filter.
+        if (nonConvertedPredList.size() == 0) {
+          call.transformTo(child);
+        } else if (nonConvertedPredList.size() < predList.size()) {


In this case, else will include both nonConvertedPredList.size() < predList.size() and nonConvertedPredList.size() == predList.size() cases, but as I pointed in the comment above, we shouldn't do anything for the last case.

vvysotskyi · 2018-11-25T11:39:10Z

exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFilterBuilder.java

@@ -63,6 +63,7 @@
  static final Logger logger = LoggerFactory.getLogger(ParquetFilterBuilder.class);

  private final UdfUtilities udfUtilities;
+  private final boolean omitUnsupportedExprs;


Added to its Javadoc case when it should be used.

…lly matches the filter

arina-ielchiieva · 2018-11-26T11:18:27Z

+1, LGTM.

…lly matches the filter closes apache#1552

arina-ielchiieva reviewed Nov 23, 2018

View reviewed changes

vvysotskyi commented Nov 23, 2018

View reviewed changes

vvysotskyi force-pushed the DRILL-6865 branch from c38ec34 to 3939d2f Compare November 23, 2018 17:46

vvysotskyi commented Nov 25, 2018

View reviewed changes

vvysotskyi added 2 commits November 26, 2018 13:13

DRILL-6865: Query returns wrong result when filter pruning happens

e553b77

DRILL-6865: Filter is not removed from the plan when parquet table fu…

747151f

…lly matches the filter

vvysotskyi force-pushed the DRILL-6865 branch from 451130a to 747151f Compare November 26, 2018 11:14

asfgit closed this in 99a3d76 Nov 26, 2018

mattpollack pushed a commit to mattpollack/drill that referenced this pull request Feb 25, 2019

DRILL-6865: Filter is not removed from the plan when parquet table fu…

ebac9b1

…lly matches the filter closes apache#1552

lushuifeng pushed a commit to lushuifeng/drill that referenced this pull request Jun 21, 2019

DRILL-6865: Filter is not removed from the plan when parquet table fu…

4871a38

…lly matches the filter closes apache#1552

xiangt920 pushed a commit to xiangt920/drill that referenced this pull request Dec 26, 2019

DRILL-6865: Filter is not removed from the plan when parquet table fu…

ff2693d

…lly matches the filter closes apache#1552

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRILL-6865: Query returns wrong result when filter pruning happens #1552

DRILL-6865: Query returns wrong result when filter pruning happens #1552

vvysotskyi commented Nov 23, 2018

arina-ielchiieva Nov 23, 2018

vvysotskyi Nov 23, 2018

arina-ielchiieva Nov 23, 2018

vvysotskyi Nov 23, 2018

arina-ielchiieva Nov 23, 2018

vvysotskyi Nov 23, 2018

arina-ielchiieva Nov 24, 2018

vvysotskyi Nov 25, 2018

arina-ielchiieva Nov 23, 2018

vvysotskyi Nov 23, 2018

arina-ielchiieva Nov 24, 2018

vvysotskyi Nov 25, 2018

vvysotskyi left a comment

vvysotskyi Nov 23, 2018

vvysotskyi Nov 23, 2018

vvysotskyi Nov 23, 2018

vvysotskyi Nov 23, 2018

vvysotskyi left a comment

vvysotskyi Nov 25, 2018

vvysotskyi Nov 25, 2018

arina-ielchiieva commented Nov 26, 2018

DRILL-6865: Query returns wrong result when filter pruning happens #1552

DRILL-6865: Query returns wrong result when filter pruning happens #1552

Conversation

vvysotskyi commented Nov 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vvysotskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vvysotskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arina-ielchiieva commented Nov 26, 2018