[BEAM-8993] [SQL] MongoDB predicate push down. #10417

11moon11 · 2019-12-18T19:35:02Z

Add a MongoDbFilter class, implementing BeamSqlTableFilter.
- Support simple comparison operations.
- Support boolean field.
- Support nested conjunction/disjunction.
Update MongoDbTable#buildIOReader
- Construct a push-down filter from RexNodes.
- Set filter to FindQuery.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang	SDK	Apex	Dataflow	Gearpump	Samza	Spark
Go		---	---	---	---
Java
Python		---		---	---
XLang	---	---	---	---	---	---

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website
Non-portable
Portable	---		---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs

11moon11 · 2019-12-18T21:19:09Z

R: @TheNeuralBit
CC: @apilloud
CC: @amaliujia

11moon11 · 2019-12-18T21:19:21Z

Run sql postcommit

TheNeuralBit

LGTM overall. I have a few architectural suggestions, the only thing I'm really firm on is that you shouldn't do the partitioning in the constructor, and if MongoDbFilter is its own class it should be package-private.

TheNeuralBit · 2020-01-07T20:54:40Z

...ql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/mongodb/MongoDbFilter.java

+        supported.add(node);
+      } else {
+        unsupported.add(node);
+      }


You shouldn't do work in a constructor. I think this constructor should accept a supported and an unsupported List and maybe be private. There could be a static initializer that will accept a single list and partition it into supported and unsupported.

Makes sense, created a static initializer.
Thanks for pointing this out!

TheNeuralBit · 2020-01-07T21:07:37Z

...rc/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/mongodb/MongoDbFilterTest.java

+        ImmutableList.of(
+            Pair.of("select * from TEST where unused1=100", true),
+            Pair.of("select * from TEST where unused1 in (100, 200)", true),
+            Pair.of("select * from TEST where b", true),
+            Pair.of("select * from TEST where not b", true),
+            // Nested conjunction and disjunction is supported as long as child operations are
+            // supported.
+            Pair.of(
+                "select * from TEST where unused1>100 and unused1<=200 and id<>1 and (name='two' or id=2)",
+                true),
+            // RegEx matching push-down is not implemented at the moment.
+            Pair.of("select * from TEST where name like 'o%e'", false),
+            // Complex operations, which modify a field before a comparison are not supported.
+            Pair.of("select * from TEST where unused1+10=110", false),
+            // Since unused2 is of type `short`, it will be cast to int32, making this a complex
+            // operation.
+            Pair.of("select * from TEST where unused2=200", false),
+            // Operations involving more than one column are not supported yet.
+            Pair.of("select * from TEST where unused1=unused2 and id=2", false),
+            Pair.of("select * from TEST where unused1+unused2=10", false));


You might consider making this a parameterized test instead, up to you though.

I agree, using parameterized tests here makes things a little cleaner and more straight forward.

TheNeuralBit · 2020-01-07T21:21:26Z

...ql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/mongodb/MongoDbFilter.java

+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.sql.SqlKind;
+import org.apache.beam.vendor.calcite.v1_20_0.org.apache.calcite.sql.type.SqlTypeName;
+
+public class MongoDbFilter implements BeamSqlTableFilter {


You might consider if you can make this an inner class of MongoDbTable, since its only used in that class' constructFilter method. If that's too big of a change, I think it should at least be package-private.

Moved MongoDbFilter.
MongoDbTable feels a little convoluted, but not too bad, I guess.
What do you think?

11moon11 · 2020-01-07T23:37:28Z

@TheNeuralBit
Could you re-run java presubmit and sql postcommit tests please?

apilloud · 2020-01-09T22:55:32Z

Run java presubmit

apilloud · 2020-01-09T22:55:44Z

Run sql postcommit

11moon11 · 2020-01-10T18:24:06Z

It looks like a test failure is due to a flake:

Could not resolve net.minidev:json-smart:[1.3.1,2.3]
... Read timed out

apilloud · 2020-01-10T18:24:29Z

Run sql postcommit

apilloud · 2020-01-10T18:24:36Z

Run JavaBeamZetaSQL PreCommit

apilloud · 2020-01-10T18:24:44Z

Run SQL PreCommit

TheNeuralBit · 2020-01-10T19:00:42Z

...sql/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/mongodb/MongoDbTable.java

 import static org.apache.beam.vendor.guava.v26_0_jre.com.google.common.base.Preconditions.checkArgument;

+import com.google.common.collect.ImmutableList;


New precommit failure is indicating you should use vendored guava here

Updated to vendored import. Thanks!

apilloud · 2020-01-10T19:51:42Z

No tests running.

apilloud · 2020-01-10T19:51:55Z

test this please

apilloud · 2020-01-10T19:52:10Z

Run precommits

apilloud · 2020-01-10T19:52:41Z

Run SQL PreCommit

TheNeuralBit · 2020-01-10T20:10:11Z

...rc/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/mongodb/MongoDbFilterTest.java

+    BeamRelNode beamRelNode = sqlEnv.parseQuery(query);
+    assertThat(beamRelNode, instanceOf(BeamCalcRel.class));
+    MongoDbFilter filter =
+        MongoDbFilter.create(((BeamCalcRel) beamRelNode).getProgram().split().right);


Doesn't necessarily need to be changed now, but I was envisioning that this would be tested through the public API, MongoDbTable#createFilter, and that MongoDbFilter could be private.

apilloud · 2020-01-10T20:57:44Z

Run SQL PreCommit

11moon11 · 2020-01-10T21:04:29Z

Run SQL PreCommit

https://builds.apache.org/view/A-D/view/Beam/view/All/job/beam_PreCommit_SQL_Phrase/8/

TheNeuralBit requested changes Jan 7, 2020

View reviewed changes

11moon11 added 5 commits January 10, 2020 10:04

Boiler plate for predicate push-down

10ce4d1

Simple filter push-down for MongoDB. Needs tests.

e2d49d0

Integration test

a04b050

Add tests for MongoDbFilter

46721b0

Refactor

9fa7ae7

TheNeuralBit reviewed Jan 10, 2020

View reviewed changes

Use vendored guava

576a06a

11moon11 force-pushed the MongoDBPredicatePushDown branch from 928f90a to 576a06a Compare January 10, 2020 19:46

apilloud assigned apilloud and unassigned apilloud Jan 10, 2020

TheNeuralBit approved these changes Jan 10, 2020

View reviewed changes

TheNeuralBit merged commit 5fedbe2 into apache:master Jan 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BEAM-8993] [SQL] MongoDB predicate push down. #10417

[BEAM-8993] [SQL] MongoDB predicate push down. #10417

11moon11 commented Dec 18, 2019 •

edited by apilloud

11moon11 commented Dec 18, 2019

11moon11 commented Dec 18, 2019

TheNeuralBit left a comment

TheNeuralBit Jan 7, 2020

11moon11 Jan 7, 2020

TheNeuralBit Jan 7, 2020

11moon11 Jan 7, 2020

TheNeuralBit Jan 7, 2020

11moon11 Jan 7, 2020

11moon11 commented Jan 7, 2020

apilloud commented Jan 9, 2020

apilloud commented Jan 9, 2020

11moon11 commented Jan 10, 2020

apilloud commented Jan 10, 2020

apilloud commented Jan 10, 2020

apilloud commented Jan 10, 2020

TheNeuralBit Jan 10, 2020

11moon11 Jan 10, 2020

apilloud commented Jan 10, 2020

apilloud commented Jan 10, 2020

apilloud commented Jan 10, 2020

apilloud commented Jan 10, 2020

TheNeuralBit Jan 10, 2020

apilloud commented Jan 10, 2020

11moon11 commented Jan 10, 2020 •

edited

		import static org.apache.beam.vendor.guava.v26_0_jre.com.google.common.base.Preconditions.checkArgument;

		import com.google.common.collect.ImmutableList;

[BEAM-8993] [SQL] MongoDB predicate push down. #10417

[BEAM-8993] [SQL] MongoDB predicate push down. #10417

Conversation

11moon11 commented Dec 18, 2019 • edited by apilloud

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

11moon11 commented Dec 18, 2019

11moon11 commented Dec 18, 2019

TheNeuralBit 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

11moon11 commented Jan 7, 2020

apilloud commented Jan 9, 2020

apilloud commented Jan 9, 2020

11moon11 commented Jan 10, 2020

apilloud commented Jan 10, 2020

apilloud commented Jan 10, 2020

apilloud commented Jan 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apilloud commented Jan 10, 2020

apilloud commented Jan 10, 2020

apilloud commented Jan 10, 2020

apilloud commented Jan 10, 2020

Choose a reason for hiding this comment

apilloud commented Jan 10, 2020

11moon11 commented Jan 10, 2020 • edited

11moon11 commented Dec 18, 2019 •

edited by apilloud

11moon11 commented Jan 10, 2020 •

edited