[WIP] StorageHandler and PredicatePushdown #11

cmathiesen · 2020-04-01T15:18:56Z

An initial PR to show the progress I've made on implementing PredicatePushdown for issue #6

Still need to add proper and comprehensive tests (although I have been running tests constantly using the HiveRunner test)
Still need to figure out this CONSTANT thing
Next step is to pass the Iceberg filter along to the Parquet Reader to use

massdosage · 2020-04-01T15:35:23Z

src/test/java/com/expediagroup/hiveberg/TestIcebergInputFormat.java

    shell.execute("CREATE DATABASE source_db");
    shell.execute(new StringBuilder()
        .append("CREATE TABLE source_db.table_a ")
-        .append("ROW FORMAT SERDE 'com.expediagroup.hiveberg.IcebergSerDe' ")


Would it be worth having a test which uses the INPUTFORMAT directly (i.e. the current test) and then another one which tests the storage handler? So we make sure that both use cases work?

Yeah, good shout

src/main/java/com/expediagroup/hiveberg/IcebergFilterFactory.java

massdosage

I think I'm going to need you to walk me through this. Will see when I can put in some time for that.

I don't know whether this will be helpful at all but I know once a upon a long time ago we did something along these lines for a custom InputFormat that we wrote for dealing with ORC files in Cascading where we also wanted to push predicates down to the underlying files. The main code for that is at https://github.com/HotelsDotCom/corc/blob/eb4fc4f462ae9243daeebaa0640d65085e3c8d98/corc-core/src/main/java/com/hotels/corc/mapred/CorcInputFormat.java and you can also look in the "sarg" package for some related classes. It might also be a good idea to ask Dave M to review this as he may remember some useful things (although it was a long time ago).

massdosage · 2020-04-06T10:16:06Z

pom.xml

      <version>${iceberg.version}</version>
      <classifier>all</classifier>
    </dependency>
+    <!-- https://mvnrepository.com/artifact/com.esotericsoftware/kryo -->


You don't need to put these links in, should be self-evident from the Maven co-ordinates.

massdosage · 2020-04-06T10:16:18Z

pom.xml

      <artifactId>avro</artifactId>
      <version>1.9.2</version>
    </dependency>
+    <!-- https://mvnrepository.com/artifact/org.apache.hive/hive-exec -->


massdosage · 2020-04-06T10:17:01Z

src/main/java/com/expediagroup/hiveberg/IcebergFilterFactory.java

+
+  IcebergFilterFactory () {}
+
+  public static Expression getFilterExpression(SearchArgument sarg) {


Suggested change

public static Expression getFilterExpression(SearchArgument sarg) {

public static Expression generateFilterExpression(SearchArgument sarg) {

cmathiesen · 2020-04-06T11:50:58Z

src/main/java/com/expediagroup/hiveberg/IcebergFilterFactory.java

+      case OR:
+        return or(recurseExpressionTree(tree.getChildren().get(0), leaves),
+            recurseExpressionTree(tree.getChildren().get(1), leaves));
+      case AND:


Handle case where there is more than 2 children from the AND operator

teabot

Would be great to also update README with feature list and limitations. Example for this PR:

Features

Storage handler implementation for simplified creation of Iceberg tables
Predicate pushdown of Hive SQL WHERE clause to file reader (Parquet only)

src/main/java/com/expediagroup/hiveberg/IcebergFilterFactory.java

teabot · 2020-04-07T12:36:42Z

src/main/java/com/expediagroup/hiveberg/IcebergStorageHandler.java

+  @Override
+  public void configureInputJobProperties(TableDesc tableDesc,
+                                          Map<String, String> jobProperties) {
+    // do nothing by default


Call super?

Suggested change

// do nothing by default

super.configureInputJobProperties(tableDesc, jobProperties);

teabot · 2020-04-07T12:37:01Z

src/main/java/com/expediagroup/hiveberg/IcebergStorageHandler.java

+  @Override
+  public void configureOutputJobProperties(TableDesc tableDesc,
+                                           Map<String, String> jobProperties) {
+    // do nothing by default


Call super?

Suggested change

// do nothing by default

super.configureOutputJobProperties(tableDesc, jobProperties);

teabot · 2020-04-07T12:37:15Z

src/main/java/com/expediagroup/hiveberg/IcebergStorageHandler.java

+  @Override
+  public void configureTableJobProperties(TableDesc tableDesc,
+                                          Map<String, String> jobProperties) {
+    //do nothing by default


Call super?

Suggested change

//do nothing by default

super.configureTableJobProperties(tableDesc, jobProperties);

teabot · 2020-04-07T12:37:59Z

src/main/java/com/expediagroup/hiveberg/IcebergStorageHandler.java

+
+  @Override
+  public void configureJobConf(TableDesc tableDesc, JobConf jobConf) {
+    //do nothing by default


Call super?

Suggested change

//do nothing by default

super.configureJobConf(tableDesc, jobConf);

teabot · 2020-04-07T12:38:31Z

src/main/java/com/expediagroup/hiveberg/IcebergStorageHandler.java

+   * @param jobConf - job configuration for InputFormat to access
+   * @param deserializer - deserializer
+   * @param exprNodeDesc - filter expression extracted by Hive
+   * @return - decomposed predicate that tells Hive what parts of the predicate are handled by the StorageHandler


Suggested change

* @return - decomposed predicate that tells Hive what parts of the predicate are handled by the StorageHandler

* @return Decomposed predicate that tells Hive what parts of the predicate are handled by the StorageHandler

teabot · 2020-04-07T12:38:59Z

src/main/java/com/expediagroup/hiveberg/IcebergStorageHandler.java

+
+  /**
+   * Extract and serialize the filter expression and add it to the Configuration for the InputFormat to access.
+   * @param jobConf - job configuration for InputFormat to access


Suggested change

* @param jobConf - job configuration for InputFormat to access

* @param jobConf Job configuration for InputFormat to access

teabot · 2020-04-07T12:39:11Z

src/main/java/com/expediagroup/hiveberg/IcebergStorageHandler.java

+  /**
+   * Extract and serialize the filter expression and add it to the Configuration for the InputFormat to access.
+   * @param jobConf - job configuration for InputFormat to access
+   * @param deserializer - deserializer


Suggested change

* @param deserializer - deserializer

* @param deserializer Deserializer

teabot · 2020-04-07T12:39:29Z

src/main/java/com/expediagroup/hiveberg/IcebergStorageHandler.java

+   * Extract and serialize the filter expression and add it to the Configuration for the InputFormat to access.
+   * @param jobConf - job configuration for InputFormat to access
+   * @param deserializer - deserializer
+   * @param exprNodeDesc - filter expression extracted by Hive


Suggested change

* @param exprNodeDesc - filter expression extracted by Hive

* @param exprNodeDesc Filter expression extracted by Hive

teabot · 2020-04-07T12:39:56Z

src/test/java/com/expediagroup/hiveberg/TestIcebergFilterFactory.java

+    assertEquals(actual.left().op(), expected.left().op());
+  }
+
+  /*@Test


Was testing out some more complex tests but haven't finished on the more comprehensive tests yet - will take out for now and probably do another PR with more tests (when i look at bigger tables + join statements etc)

teabot · 2020-04-07T12:56:18Z

src/main/java/com/expediagroup/hiveberg/IcebergReaderFactory.java


-  public CloseableIterable<Record> createReader(DataFile file, FileScanTask currentTask, InputFile inputFile, Schema tableSchema, boolean reuseContainers) {
+  public CloseableIterable<Record> createReader(DataFile file, FileScanTask currentTask, InputFile inputFile,
+                                                Schema tableSchema, boolean reuseContainers) {


For ORC reader, perhaps add a comment advising the reader to look here to determine when this can be wired up for ORC: apache/iceberg#787

cmathiesen · 2020-04-07T14:02:30Z

Would be great to also update README with feature list and limitations. Example for this PR:

Features

Storage handler implementation for simplified creation of Iceberg tables

Predicate pushdown of Hive SQL WHERE clause to file reader (Parquet only)

Addressed your other review comments, going to work on an update to the README then will ping again for a final check

massdosage · 2020-04-07T14:44:59Z

README.md

+## Features
+### IcebergInputFormat
+
+To use the `IcebergInputFormat` you need to write a Hive query similar to:


Suggested change

To use the `IcebergInputFormat` you need to write a Hive query similar to:

To use the `IcebergInputFormat` you need to create a Hive table using DDL similar to:

massdosage · 2020-04-07T14:45:20Z

README.md

+```
+
+### IcebergStorageHandler
+This is implemented as a simplified option for creating Hiveberg tables. The Hive query should instead look like:


Suggested change

This is implemented as a simplified option for creating Hiveberg tables. The Hive query should instead look like:

This is implemented as a simplified option for creating Hiveberg tables. The Hive DDL should instead look like:

massdosage · 2020-04-07T14:45:53Z

README.md

+```
+
+### Predicate Pushdown
+Pushdown of the HiveSQL `WHERE` clause has been implemented so that filter's are pushed to the Iceberg `TableScan` level as well as the Parquet `Reader`. ORC implementations are still in the works. 


Suggested change

Pushdown of the HiveSQL `WHERE` clause has been implemented so that filter's are pushed to the Iceberg `TableScan` level as well as the Parquet `Reader`. ORC implementations are still in the works.

Pushdown of the HiveSQL `WHERE` clause has been implemented so that filters are pushed to the Iceberg `TableScan` level as well as the Parquet `Reader`. ORC implementations are still in the works.

massdosage · 2020-04-07T14:46:49Z

pom.xml

      <groupId>org.apache.hive</groupId>
      <artifactId>hive-serde</artifactId>
-      <version>2.3.4</version>
+      <version>2.3.6</version>


Let's create a hive.version property up above and reference it here and on line 78 so we only have one place to update it in future.

src/main/java/com/expediagroup/hiveberg/IcebergFilterFactory.java

massdosage · 2020-04-07T14:49:40Z

src/main/java/com/expediagroup/hiveberg/IcebergFilterFactory.java

+
+public class IcebergFilterFactory {
+
+  IcebergFilterFactory () {}


Can this be made private?

massdosage · 2020-04-07T14:50:43Z

src/main/java/com/expediagroup/hiveberg/IcebergFilterFactory.java

+
+  /**
+   * Remove first 2 nodes already evaluated and return an array of the evaluated leftover nodes.
+   * @param allChildNodes - all child nodes to be evaluated for the AND expression.


Suggested change

* @param allChildNodes - all child nodes to be evaluated for the AND expression.

* @param allChildNodes All child nodes to be evaluated for the AND expression.

(applies to all other javadoc in this PR, remove the - and capitalise the first letter)

src/main/java/com/expediagroup/hiveberg/IcebergFilterFactory.java

massdosage · 2020-04-08T09:17:07Z

src/main/java/com/expediagroup/hiveberg/IcebergFilterFactory.java

@@ -0,0 +1,153 @@
+/**
+ * Copyright (C) 2020 Expedia, Inc and the Apache Software Foundation.


Suggested change

* Copyright (C) 2020 Expedia, Inc and the Apache Software Foundation.

* Copyright (C) 2020 Expedia, Inc. and the Apache Software Foundation.

massdosage · 2020-04-08T09:19:03Z

src/test/java/com/expediagroup/hiveberg/TestIcebergFilterFactory.java

+ */
+package com.expediagroup.hiveberg;
+
+import static org.junit.Assert.assertEquals;


static needs to go after non-static

massdosage

Two very small "nits", rest looks ready to go.

cmathiesen added 2 commits April 1, 2020 15:41

Add IcebergStorageHandler and predicate pushdown initial attempt

6191059

Add constant options

5d66e93

cmathiesen requested review from massdosage and teabot and removed request for massdosage April 1, 2020 15:19

massdosage reviewed Apr 1, 2020

View reviewed changes

teabot reviewed Apr 1, 2020

View reviewed changes

src/main/java/com/expediagroup/hiveberg/IcebergFilterFactory.java Outdated Show resolved Hide resolved

teabot reviewed Apr 1, 2020

View reviewed changes

src/main/java/com/expediagroup/hiveberg/IcebergFilterFactory.java Outdated Show resolved Hide resolved

teabot reviewed Apr 1, 2020

View reviewed changes

src/main/java/com/expediagroup/hiveberg/IcebergFilterFactory.java Outdated Show resolved Hide resolved

cmathiesen added 4 commits April 2, 2020 11:20

Apply suggests from review

fb567a7

Add tests and fix AND to work for unlimited list of leaves

b683216

Push filter to the Parquet reader and some clean up

f0fb336

Revert version change

9d11a1c

massdosage reviewed Apr 6, 2020

View reviewed changes

cmathiesen commented Apr 6, 2020

View reviewed changes

cmathiesen added 2 commits April 6, 2020 17:04

clean up

84b231f

fix BETWEEN operator conversion

1380857

teabot approved these changes Apr 7, 2020

View reviewed changes

teabot reviewed Apr 7, 2020

View reviewed changes

Address review comments

7ac9972

Update to README

1cf711e

cmathiesen requested a review from massdosage April 7, 2020 14:24

massdosage suggested changes Apr 7, 2020

View reviewed changes

Fix javadoc and reference in license

c784cf3

cmathiesen requested a review from massdosage April 8, 2020 08:40

massdosage reviewed Apr 8, 2020

View reviewed changes

massdosage suggested changes Apr 8, 2020

View reviewed changes

Picking nits

b663318

massdosage approved these changes Apr 8, 2020

View reviewed changes

cmathiesen merged commit 98ab559 into master Apr 8, 2020

cmathiesen deleted the predicate-pushdown branch April 8, 2020 09:54


		IcebergFilterFactory () {}

		public static Expression getFilterExpression(SearchArgument sarg) {

	public static Expression getFilterExpression(SearchArgument sarg) {
	public static Expression generateFilterExpression(SearchArgument sarg) {

	// do nothing by default
	super.configureInputJobProperties(tableDesc, jobProperties);

	// do nothing by default
	super.configureOutputJobProperties(tableDesc, jobProperties);

	//do nothing by default
	super.configureTableJobProperties(tableDesc, jobProperties);

	//do nothing by default
	super.configureJobConf(tableDesc, jobConf);

	* @return - decomposed predicate that tells Hive what parts of the predicate are handled by the StorageHandler
	* @return Decomposed predicate that tells Hive what parts of the predicate are handled by the StorageHandler

	* @param jobConf - job configuration for InputFormat to access
	* @param jobConf Job configuration for InputFormat to access

	* @param deserializer - deserializer
	* @param deserializer Deserializer

	* @param exprNodeDesc - filter expression extracted by Hive
	* @param exprNodeDesc Filter expression extracted by Hive

	To use the `IcebergInputFormat` you need to write a Hive query similar to:
	To use the `IcebergInputFormat` you need to create a Hive table using DDL similar to:

	This is implemented as a simplified option for creating Hiveberg tables. The Hive query should instead look like:
	This is implemented as a simplified option for creating Hiveberg tables. The Hive DDL should instead look like:

	Pushdown of the HiveSQL `WHERE` clause has been implemented so that filter's are pushed to the Iceberg `TableScan` level as well as the Parquet `Reader`. ORC implementations are still in the works.
	Pushdown of the HiveSQL `WHERE` clause has been implemented so that filters are pushed to the Iceberg `TableScan` level as well as the Parquet `Reader`. ORC implementations are still in the works.


		public class IcebergFilterFactory {

		IcebergFilterFactory () {}

	* @param allChildNodes - all child nodes to be evaluated for the AND expression.
	* @param allChildNodes All child nodes to be evaluated for the AND expression.

		@@ -0,0 +1,153 @@
		/**
		* Copyright (C) 2020 Expedia, Inc and the Apache Software Foundation.

	* Copyright (C) 2020 Expedia, Inc and the Apache Software Foundation.
	* Copyright (C) 2020 Expedia, Inc. and the Apache Software Foundation.

[WIP] StorageHandler and PredicatePushdown #11

[WIP] StorageHandler and PredicatePushdown #11

Uh oh!

Conversation

cmathiesen commented Apr 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

massdosage left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teabot left a comment

Choose a reason for hiding this comment

Features

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmathiesen commented Apr 7, 2020

Features

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

massdosage left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

cmathiesen commented Apr 1, 2020 •

edited

Loading