You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "SteNicholas (via GitHub)" <gi...@apache.org> on 2023/02/15 03:31:04 UTC
[GitHub] [flink-table-store] SteNicholas opened a new pull request, #537: [FLINK-31050] Supports IN and NOT IN predicate in ORC format of table store
SteNicholas opened a new pull request, #537:
URL: https://github.com/apache/flink-table-store/pull/537
Supports `IN` and `NOT IN` predicate push down in ORC format for `OrcPredicateFunctionVisitor`.
**The brief change log**
- Supports `visitIn` and `visitNotIn` in `OrcPredicateFunctionVisitor`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] SteNicholas commented on a diff in pull request #537: [FLINK-31050] Supports IN and NOT IN predicate in ORC format of table store
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #537:
URL: https://github.com/apache/flink-table-store/pull/537#discussion_r1106621319
##########
flink-table-store-format/src/main/java/org/apache/flink/table/store/format/orc/filter/OrcPredicateFunctionVisitor.java:
##########
@@ -157,20 +158,34 @@ private Optional<OrcFilters.Predicate> convertBinary(
if (litType == null) {
return Optional.empty();
}
-
- String colName = fieldRef.name();
-
// fetch literal and ensure it is serializable
Object orcObj = toOrcObject(litType, literal);
- Serializable serializableLiteral;
// validate that literal is serializable
- if (orcObj instanceof Serializable) {
- serializableLiteral = (Serializable) orcObj;
- } else {
+ return orcObj instanceof Serializable
+ ? Optional.of(func.apply(fieldRef.name(), litType, (Serializable) orcObj))
+ : Optional.empty();
+ }
+
+ private Optional<OrcFilters.Predicate> convertIn(FieldRef fieldRef, List<Object> literals) {
+ PredicateLeaf.Type litType = toOrcType(fieldRef.type());
+ if (litType == null || CollectionUtils.isEmpty(literals)) {
return Optional.empty();
}
-
- return Optional.of(func.apply(colName, litType, serializableLiteral));
+ return Optional.of(
+ new OrcFilters.In(
+ fieldRef.name(),
+ litType,
+ literals.stream()
+ .map(
+ literal -> {
+ // fetch literal and ensure it is serializable
+ Object orcObj = toOrcObject(litType, literal);
+ // validate that literal is serializable
+ return orcObj instanceof Serializable
+ ? (Serializable) orcObj
+ : null;
Review Comment:
@FangYongs, IMO, it's better to filter serializable literal for the literals, and don't care the non-serializable literal. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] SteNicholas commented on a diff in pull request #537: [FLINK-31050] Supports IN and NOT IN predicate in ORC format of table store
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #537:
URL: https://github.com/apache/flink-table-store/pull/537#discussion_r1106646269
##########
flink-table-store-format/src/main/java/org/apache/flink/table/store/format/orc/filter/OrcPredicateFunctionVisitor.java:
##########
@@ -157,20 +158,34 @@ private Optional<OrcFilters.Predicate> convertBinary(
if (litType == null) {
return Optional.empty();
}
-
- String colName = fieldRef.name();
-
// fetch literal and ensure it is serializable
Object orcObj = toOrcObject(litType, literal);
- Serializable serializableLiteral;
// validate that literal is serializable
- if (orcObj instanceof Serializable) {
- serializableLiteral = (Serializable) orcObj;
- } else {
+ return orcObj instanceof Serializable
+ ? Optional.of(func.apply(fieldRef.name(), litType, (Serializable) orcObj))
+ : Optional.empty();
+ }
+
+ private Optional<OrcFilters.Predicate> convertIn(FieldRef fieldRef, List<Object> literals) {
+ PredicateLeaf.Type litType = toOrcType(fieldRef.type());
+ if (litType == null || CollectionUtils.isEmpty(literals)) {
return Optional.empty();
}
-
- return Optional.of(func.apply(colName, litType, serializableLiteral));
+ return Optional.of(
+ new OrcFilters.In(
+ fieldRef.name(),
+ litType,
+ literals.stream()
+ .map(
+ literal -> {
+ // fetch literal and ensure it is serializable
+ Object orcObj = toOrcObject(litType, literal);
+ // validate that literal is serializable
+ return orcObj instanceof Serializable
+ ? (Serializable) orcObj
+ : null;
Review Comment:
Make sense to me. Keeps the consistence with the non-serializable object of `convertBinary`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] JingsongLi merged pull request #537: [FLINK-31050] Add cases for IN and NOT IN predicate in ORC format
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #537:
URL: https://github.com/apache/flink-table-store/pull/537
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] SteNicholas commented on a diff in pull request #537: [FLINK-31050] Supports IN and NOT IN predicate in ORC format of table store
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #537:
URL: https://github.com/apache/flink-table-store/pull/537#discussion_r1106621319
##########
flink-table-store-format/src/main/java/org/apache/flink/table/store/format/orc/filter/OrcPredicateFunctionVisitor.java:
##########
@@ -157,20 +158,34 @@ private Optional<OrcFilters.Predicate> convertBinary(
if (litType == null) {
return Optional.empty();
}
-
- String colName = fieldRef.name();
-
// fetch literal and ensure it is serializable
Object orcObj = toOrcObject(litType, literal);
- Serializable serializableLiteral;
// validate that literal is serializable
- if (orcObj instanceof Serializable) {
- serializableLiteral = (Serializable) orcObj;
- } else {
+ return orcObj instanceof Serializable
+ ? Optional.of(func.apply(fieldRef.name(), litType, (Serializable) orcObj))
+ : Optional.empty();
+ }
+
+ private Optional<OrcFilters.Predicate> convertIn(FieldRef fieldRef, List<Object> literals) {
+ PredicateLeaf.Type litType = toOrcType(fieldRef.type());
+ if (litType == null || CollectionUtils.isEmpty(literals)) {
return Optional.empty();
}
-
- return Optional.of(func.apply(colName, litType, serializableLiteral));
+ return Optional.of(
+ new OrcFilters.In(
+ fieldRef.name(),
+ litType,
+ literals.stream()
+ .map(
+ literal -> {
+ // fetch literal and ensure it is serializable
+ Object orcObj = toOrcObject(litType, literal);
+ // validate that literal is serializable
+ return orcObj instanceof Serializable
+ ? (Serializable) orcObj
+ : null;
Review Comment:
@FangYongs, IMO, it's better to filter serializable literal for the literals, and don't care the non-serializable literal. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] FangYongs commented on a diff in pull request #537: [FLINK-31050] Supports IN and NOT IN predicate in ORC format of table store
Posted by "FangYongs (via GitHub)" <gi...@apache.org>.
FangYongs commented on code in PR #537:
URL: https://github.com/apache/flink-table-store/pull/537#discussion_r1106618203
##########
flink-table-store-format/src/main/java/org/apache/flink/table/store/format/orc/filter/OrcPredicateFunctionVisitor.java:
##########
@@ -157,20 +158,34 @@ private Optional<OrcFilters.Predicate> convertBinary(
if (litType == null) {
return Optional.empty();
}
-
- String colName = fieldRef.name();
-
// fetch literal and ensure it is serializable
Object orcObj = toOrcObject(litType, literal);
- Serializable serializableLiteral;
// validate that literal is serializable
- if (orcObj instanceof Serializable) {
- serializableLiteral = (Serializable) orcObj;
- } else {
+ return orcObj instanceof Serializable
+ ? Optional.of(func.apply(fieldRef.name(), litType, (Serializable) orcObj))
+ : Optional.empty();
+ }
+
+ private Optional<OrcFilters.Predicate> convertIn(FieldRef fieldRef, List<Object> literals) {
+ PredicateLeaf.Type litType = toOrcType(fieldRef.type());
+ if (litType == null || CollectionUtils.isEmpty(literals)) {
return Optional.empty();
}
-
- return Optional.of(func.apply(colName, litType, serializableLiteral));
+ return Optional.of(
+ new OrcFilters.In(
+ fieldRef.name(),
+ litType,
+ literals.stream()
+ .map(
+ literal -> {
+ // fetch literal and ensure it is serializable
+ Object orcObj = toOrcObject(litType, literal);
+ // validate that literal is serializable
+ return orcObj instanceof Serializable
+ ? (Serializable) orcObj
+ : null;
Review Comment:
Should we put `null` in literals results or return `Optional.empty` for `convertIn`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] FangYongs commented on pull request #537: [FLINK-31050] Supports IN and NOT IN predicate in ORC format of table store
Posted by "FangYongs (via GitHub)" <gi...@apache.org>.
FangYongs commented on PR #537:
URL: https://github.com/apache/flink-table-store/pull/537#issuecomment-1430784498
Thanks @SteNicholas , LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-table-store] SteNicholas commented on pull request #537: [FLINK-31050] Supports IN and NOT IN predicate in ORC format of table store
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on PR #537:
URL: https://github.com/apache/flink-table-store/pull/537#issuecomment-1430837677
@JingsongLi, @FangYongs, according to above comment, I have removed the `Between` and `In` of `OrcFilters` and added the comment for 20 literals performance critical. PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org