You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by fhueske <gi...@git.apache.org> on 2018/01/23 21:14:49 UTC
[GitHub] flink pull request #5345: [FLINK-8242] [table] Fix predicate push-down of Or...
GitHub user fhueske opened a pull request:
https://github.com/apache/flink/pull/5345
[FLINK-8242] [table] Fix predicate push-down of OrcTableSource
## What is the purpose of the change
* Fix `ClassClassException` in `OrcTableSource` caused by hard cast to `Serializable`
* Fix translation of Calcite literals with `NlString`
## Brief change log
* add check to `OrcTableSource` before casting to `Serializable`
* add logging to `OrcTableSource` for predicate push-down
* add fallback for Calcite literal translation for `NlString` literals
## Verifying this change
* adds a test to `OrcTableSourceTest.testApplyPredicate()`
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): **no**
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: **no**
- The serializers: **no**
- The runtime per-record code paths (performance sensitive): **no**
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: **no**
- The S3 file system connector: **no**
## Documentation
- Does this pull request introduce a new feature? **no**
- If yes, how is the feature documented? **n/a**
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/fhueske/flink orcPushDown
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/5345.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #5345
----
commit 796deed1fe6372ca2eafc1f3b08387b7f29d5841
Author: Fabian Hueske <fh...@...>
Date: 2017-12-12T21:38:51Z
[FLINK-8242] [orc] Fix predicate translation if literal is not Serializable.
commit 07b8fe1eaf68e175c963048980809a4ce4605643
Author: Fabian Hueske <fh...@...>
Date: 2018-01-23T21:10:00Z
[FLINK-8242] [table] RexProgramExtractor convert NlsString literal always into String.
----
---
[GitHub] flink issue #5345: [FLINK-8242] [orc] Fix predicate push-down of OrcTableSou...
Posted by twalthr <gi...@git.apache.org>.
Github user twalthr commented on the issue:
https://github.com/apache/flink/pull/5345
Thank you @fhueske. The code looks good now. I will merge this...
---
[GitHub] flink pull request #5345: [FLINK-8242] [orc] Fix predicate push-down of OrcT...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/5345
---
[GitHub] flink pull request #5345: [FLINK-8242] [orc] Fix predicate push-down of OrcT...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/5345#discussion_r164084612
--- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/table/plan/util/RexProgramExtractor.scala ---
@@ -214,6 +213,10 @@ class RexNodeToExpressionConverter(
// convert to BigDecimal
literal.getValueAs(classOf[java.math.BigDecimal])
+ case _ if literal.getValue.isInstanceOf[NlsString] =>
--- End diff --
Yes, you are right. This case should not have happened but was reported by a user.
Since the Rex translation is only used for predicate push-down, we could play safe and drop the catch-all case at the end and write a debug statement that includes the value and type of the unsupported literal. Most TableSources with predicate push-down support will only support simple predicates anyway.
---
[GitHub] flink pull request #5345: [FLINK-8242] [orc] Fix predicate push-down of OrcT...
Posted by twalthr <gi...@git.apache.org>.
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/5345#discussion_r163517596
--- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/table/plan/util/RexProgramExtractor.scala ---
@@ -214,6 +213,10 @@ class RexNodeToExpressionConverter(
// convert to BigDecimal
literal.getValueAs(classOf[java.math.BigDecimal])
+ case _ if literal.getValue.isInstanceOf[NlsString] =>
--- End diff --
This should not be necessary because it is already done line 204. All tests are still working if I remove this change.
---
[GitHub] flink issue #5345: [FLINK-8242] [orc] Fix predicate push-down of OrcTableSou...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/5345
Merging
---
[GitHub] flink issue #5345: [FLINK-8242] [orc] Fix predicate push-down of OrcTableSou...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/5345
Thanks for the review @twalthr.
I've updated the PR.
---