You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/07/01 09:49:25 UTC
[GitHub] [doris] SaintBacchus opened a new pull request, #10547: [External Table]Fix hive partiton prune in hive and hudi exteranl table.
SaintBacchus opened a new pull request, #10547:
URL: https://github.com/apache/doris/pull/10547
# Proposed changes
Issue Number: close #10546
## Problem Summary:
The hive ExprBuilder always return the binary Expr with col and value order.
The exprs `col >= 1` and `1 <= col` is the same expr, but now treat it as `col >= 1` and `col <= 1`.
So it will cause this problem in the issue.
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
2. Has unit tests been added: (Yes/No/No Need)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (Yes/No)
5. Are there any changes that cannot be rolled back: (Yes/No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #10547: [multi-catalog]Fix hive partiton prune in hive and hudi exteranl table.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10547:
URL: https://github.com/apache/doris/pull/10547#issuecomment-1175704965
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman commented on a diff in pull request #10547: [multi-catalog]Fix hive partiton prune in hive and hudi exteranl table.
Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #10547:
URL: https://github.com/apache/doris/pull/10547#discussion_r913425447
##########
fe/fe-core/src/main/java/org/apache/doris/planner/HudiScanNode.java:
##########
@@ -229,41 +225,11 @@ private void initParams(Analyzer analyzer) {
context.slotDescByName = slotDescByName;
}
-
- /**
- * Extracts partition predicate from SelectStmt.whereClause that can be pushed down to Hive.
- */
- private void extractHivePartitionPredicate() throws DdlException {
- ListIterator<Expr> it = conjuncts.listIterator();
- while (it.hasNext()) {
- ExprNodeGenericFuncDesc hiveExpr = HiveMetaStoreClientHelper.convertToHivePartitionExpr(
- it.next(), partitionKeys, hudiTable.getName());
- if (hiveExpr != null) {
- hivePredicates.add(hiveExpr);
- }
- }
- int count = hivePredicates.size();
- // combine all predicate by `and`
- // compoundExprs must have at least 2 predicates
- if (count >= 2) {
- hivePartitionPredicate = HiveMetaStoreClientHelper.getCompoundExpr(hivePredicates, "and");
- } else if (count == 1) {
- // only one predicate
- hivePartitionPredicate = (ExprNodeGenericFuncDesc) hivePredicates.get(0);
- } else {
- // have no predicate, make a dummy predicate "1=1" to get all partitions
- HiveMetaStoreClientHelper.ExprBuilder exprBuilder =
- new HiveMetaStoreClientHelper.ExprBuilder(hudiTable.getName());
- hivePartitionPredicate = exprBuilder.val(TypeInfoFactory.intTypeInfo, 1)
- .val(TypeInfoFactory.intTypeInfo, 1)
- .pred("=", 2).build();
- }
- }
-
private InputSplit[] getSplits() throws UserException, IOException {
String splitsPath = basePath;
if (partitionKeys.size() > 0) {
- extractHivePartitionPredicate();
+ hivePartitionPredicate = HiveMetaStoreClientHelper.convertToHivePartitionExpr(
Review Comment:
We call `convertToHivePartitionExpr` twice in `HudiScanNode`.
I think the call in `getFileStatus()` can be removed?
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/HiveMetaStoreClientHelper.java:
##########
@@ -400,61 +401,123 @@ public static List<FieldSchema> getSchema(String dbName, String tableName, Strin
/**
* Convert Doris expr to Hive expr, only for partition column
- * @param dorisExpr
- * @param partitions
* @param tblName
* @return
* @throws DdlException
* @throws SemanticException
*/
- public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(Expr dorisExpr,
+ public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(List<Expr> conjuncts,
+ List<String> partitionKeys, String tblName) throws DdlException {
+ List<ExprNodeDesc> hivePredicates = new ArrayList<>();
+
+ for (Expr conjunct : conjuncts) {
+ ExprNodeGenericFuncDesc hiveExpr = HiveMetaStoreClientHelper.convertToHivePartitionExpr(
+ conjunct, partitionKeys, tblName).getFuncDesc();
+ if (hiveExpr != null) {
+ hivePredicates.add(hiveExpr);
+ }
+ }
+ int count = hivePredicates.size();
+ // combine all predicate by `and`
+ // compoundExprs must have at least 2 predicates
+ if (count >= 2) {
+ return HiveMetaStoreClientHelper.getCompoundExpr(hivePredicates, "and");
+ } else if (count == 1) {
+ // only one predicate
+ return (ExprNodeGenericFuncDesc) hivePredicates.get(0);
+ } else {
+ return genAlwaysTrueExpr(tblName);
+ }
+ }
+
+ private static ExprNodeGenericFuncDesc genAlwaysTrueExpr(String tblName) throws DdlException {
+ // have no predicate, make a dummy predicate "1=1" to get all partitions
+ HiveMetaStoreClientHelper.ExprBuilder exprBuilder =
+ new HiveMetaStoreClientHelper.ExprBuilder(tblName);
+ return exprBuilder.val(TypeInfoFactory.intTypeInfo, 1)
+ .val(TypeInfoFactory.intTypeInfo, 1)
+ .pred("=", 2).build();
+ }
+
+ private static class ExprNodeGenericFuncDescContext {
+ private final ExprNodeGenericFuncDesc funcDesc;
+ private final boolean eligible;
+
+ public ExprNodeGenericFuncDescContext(ExprNodeGenericFuncDesc funcDesc) {
+ this.funcDesc = funcDesc;
+ this.eligible = true;
+ }
+
+ public ExprNodeGenericFuncDescContext() {
+ this.funcDesc = null;
+ this.eligible = false;
+ }
+
+ /**
+ * Check eligible before use the expr in CompoundPredicate for `and` and `or` .
+ */
+ public boolean isEligible() {
+ return eligible;
+ }
+
+ public ExprNodeGenericFuncDesc getFuncDesc() {
+ return funcDesc;
+ }
+ }
+
+ private static ExprNodeGenericFuncDescContext convertToHivePartitionExpr(Expr dorisExpr,
List<String> partitions, String tblName) throws DdlException {
if (dorisExpr == null) {
- return null;
+ return new ExprNodeGenericFuncDescContext();
}
if (dorisExpr instanceof CompoundPredicate) {
CompoundPredicate compoundPredicate = (CompoundPredicate) dorisExpr;
switch (compoundPredicate.getOp()) {
case AND: {
- ExprNodeGenericFuncDesc left = convertToHivePartitionExpr(
- compoundPredicate.getChild(0), partitions, tblName);
- ExprNodeGenericFuncDesc right = convertToHivePartitionExpr(
+ ExprNodeGenericFuncDescContext left = convertToHivePartitionExpr(
compoundPredicate.getChild(0), partitions, tblName);
- if (left != null && right != null) {
+ ExprNodeGenericFuncDescContext right = convertToHivePartitionExpr(
+ compoundPredicate.getChild(1), partitions, tblName);
+
+ if (left.isEligible() && right.isEligible()) {
List<ExprNodeDesc> andArgs = new ArrayList<>();
- andArgs.add(left);
- andArgs.add(right);
- return getCompoundExpr(andArgs, "and");
- } else if (left != null && right == null) {
+ andArgs.add(left.getFuncDesc());
+ andArgs.add(right.getFuncDesc());
+ return new ExprNodeGenericFuncDescContext(getCompoundExpr(andArgs, "and"));
+ } else if (left.isEligible()) {
return left;
- } else if (left == null && right != null) {
+ } else if (right.isEligible()) {
return right;
+ } else {
+ return new ExprNodeGenericFuncDescContext();
}
- return null;
}
case OR: {
- ExprNodeGenericFuncDesc left = convertToHivePartitionExpr(
- compoundPredicate.getChild(0), partitions, tblName);
- ExprNodeGenericFuncDesc right = convertToHivePartitionExpr(
+ ExprNodeGenericFuncDescContext left = convertToHivePartitionExpr(
compoundPredicate.getChild(0), partitions, tblName);
- if (left != null && right != null) {
- List<ExprNodeDesc> orArgs = new ArrayList<>();
- orArgs.add(left);
- orArgs.add(right);
- return getCompoundExpr(orArgs, "or");
- } else if (left != null && right == null) {
- return left;
- } else if (left == null && right != null) {
- return right;
+ ExprNodeGenericFuncDescContext right = convertToHivePartitionExpr(
+ compoundPredicate.getChild(1), partitions, tblName);
+ if (left.isEligible() && right.isEligible()) {
+ List<ExprNodeDesc> andArgs = new ArrayList<>();
+ andArgs.add(left.getFuncDesc());
+ andArgs.add(right.getFuncDesc());
+ return new ExprNodeGenericFuncDescContext(getCompoundExpr(andArgs, "or"));
+ } else {
+ // If it is not a partition key, this is an always true expr.
+ // Or if is a partition key and also is a not supportedOp, this is an always true expr.
+ return new ExprNodeGenericFuncDescContext(genAlwaysTrueExpr(tblName));
}
- return null;
}
default:
- return null;
+ return new ExprNodeGenericFuncDescContext();
}
}
+ return binaryExprDesc(dorisExpr, partitions, tblName);
+ }
+ private static ExprNodeGenericFuncDescContext binaryExprDesc(Expr dorisExpr,
+ List<String> partitions, String tblName) throws DdlException {
Review Comment:
```suggestion
List<String> partitionKeys, String tblName) throws DdlException {
```
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/HiveMetaStoreClientHelper.java:
##########
@@ -400,61 +401,123 @@ public static List<FieldSchema> getSchema(String dbName, String tableName, Strin
/**
* Convert Doris expr to Hive expr, only for partition column
- * @param dorisExpr
- * @param partitions
* @param tblName
* @return
* @throws DdlException
* @throws SemanticException
*/
- public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(Expr dorisExpr,
+ public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(List<Expr> conjuncts,
+ List<String> partitionKeys, String tblName) throws DdlException {
+ List<ExprNodeDesc> hivePredicates = new ArrayList<>();
+
+ for (Expr conjunct : conjuncts) {
+ ExprNodeGenericFuncDesc hiveExpr = HiveMetaStoreClientHelper.convertToHivePartitionExpr(
+ conjunct, partitionKeys, tblName).getFuncDesc();
+ if (hiveExpr != null) {
+ hivePredicates.add(hiveExpr);
+ }
+ }
+ int count = hivePredicates.size();
+ // combine all predicate by `and`
+ // compoundExprs must have at least 2 predicates
+ if (count >= 2) {
+ return HiveMetaStoreClientHelper.getCompoundExpr(hivePredicates, "and");
+ } else if (count == 1) {
+ // only one predicate
+ return (ExprNodeGenericFuncDesc) hivePredicates.get(0);
+ } else {
+ return genAlwaysTrueExpr(tblName);
+ }
+ }
+
+ private static ExprNodeGenericFuncDesc genAlwaysTrueExpr(String tblName) throws DdlException {
+ // have no predicate, make a dummy predicate "1=1" to get all partitions
+ HiveMetaStoreClientHelper.ExprBuilder exprBuilder =
+ new HiveMetaStoreClientHelper.ExprBuilder(tblName);
+ return exprBuilder.val(TypeInfoFactory.intTypeInfo, 1)
+ .val(TypeInfoFactory.intTypeInfo, 1)
+ .pred("=", 2).build();
+ }
+
+ private static class ExprNodeGenericFuncDescContext {
+ private final ExprNodeGenericFuncDesc funcDesc;
+ private final boolean eligible;
+
+ public ExprNodeGenericFuncDescContext(ExprNodeGenericFuncDesc funcDesc) {
+ this.funcDesc = funcDesc;
+ this.eligible = true;
+ }
+
+ public ExprNodeGenericFuncDescContext() {
Review Comment:
We can use a `static final` field to replace this constructor.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/HiveMetaStoreClientHelper.java:
##########
@@ -400,61 +401,123 @@ public static List<FieldSchema> getSchema(String dbName, String tableName, Strin
/**
* Convert Doris expr to Hive expr, only for partition column
- * @param dorisExpr
- * @param partitions
* @param tblName
* @return
* @throws DdlException
* @throws SemanticException
*/
- public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(Expr dorisExpr,
+ public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(List<Expr> conjuncts,
+ List<String> partitionKeys, String tblName) throws DdlException {
+ List<ExprNodeDesc> hivePredicates = new ArrayList<>();
+
+ for (Expr conjunct : conjuncts) {
+ ExprNodeGenericFuncDesc hiveExpr = HiveMetaStoreClientHelper.convertToHivePartitionExpr(
+ conjunct, partitionKeys, tblName).getFuncDesc();
+ if (hiveExpr != null) {
+ hivePredicates.add(hiveExpr);
+ }
+ }
+ int count = hivePredicates.size();
+ // combine all predicate by `and`
+ // compoundExprs must have at least 2 predicates
+ if (count >= 2) {
+ return HiveMetaStoreClientHelper.getCompoundExpr(hivePredicates, "and");
+ } else if (count == 1) {
+ // only one predicate
+ return (ExprNodeGenericFuncDesc) hivePredicates.get(0);
+ } else {
+ return genAlwaysTrueExpr(tblName);
+ }
+ }
+
+ private static ExprNodeGenericFuncDesc genAlwaysTrueExpr(String tblName) throws DdlException {
+ // have no predicate, make a dummy predicate "1=1" to get all partitions
+ HiveMetaStoreClientHelper.ExprBuilder exprBuilder =
+ new HiveMetaStoreClientHelper.ExprBuilder(tblName);
+ return exprBuilder.val(TypeInfoFactory.intTypeInfo, 1)
+ .val(TypeInfoFactory.intTypeInfo, 1)
+ .pred("=", 2).build();
+ }
+
+ private static class ExprNodeGenericFuncDescContext {
+ private final ExprNodeGenericFuncDesc funcDesc;
+ private final boolean eligible;
+
+ public ExprNodeGenericFuncDescContext(ExprNodeGenericFuncDesc funcDesc) {
+ this.funcDesc = funcDesc;
+ this.eligible = true;
+ }
+
+ public ExprNodeGenericFuncDescContext() {
+ this.funcDesc = null;
+ this.eligible = false;
+ }
+
+ /**
+ * Check eligible before use the expr in CompoundPredicate for `and` and `or` .
+ */
+ public boolean isEligible() {
+ return eligible;
+ }
+
+ public ExprNodeGenericFuncDesc getFuncDesc() {
+ return funcDesc;
+ }
+ }
+
+ private static ExprNodeGenericFuncDescContext convertToHivePartitionExpr(Expr dorisExpr,
List<String> partitions, String tblName) throws DdlException {
if (dorisExpr == null) {
- return null;
+ return new ExprNodeGenericFuncDescContext();
}
if (dorisExpr instanceof CompoundPredicate) {
CompoundPredicate compoundPredicate = (CompoundPredicate) dorisExpr;
switch (compoundPredicate.getOp()) {
case AND: {
- ExprNodeGenericFuncDesc left = convertToHivePartitionExpr(
- compoundPredicate.getChild(0), partitions, tblName);
- ExprNodeGenericFuncDesc right = convertToHivePartitionExpr(
+ ExprNodeGenericFuncDescContext left = convertToHivePartitionExpr(
compoundPredicate.getChild(0), partitions, tblName);
- if (left != null && right != null) {
+ ExprNodeGenericFuncDescContext right = convertToHivePartitionExpr(
+ compoundPredicate.getChild(1), partitions, tblName);
+
+ if (left.isEligible() && right.isEligible()) {
List<ExprNodeDesc> andArgs = new ArrayList<>();
- andArgs.add(left);
- andArgs.add(right);
- return getCompoundExpr(andArgs, "and");
- } else if (left != null && right == null) {
+ andArgs.add(left.getFuncDesc());
+ andArgs.add(right.getFuncDesc());
+ return new ExprNodeGenericFuncDescContext(getCompoundExpr(andArgs, "and"));
+ } else if (left.isEligible()) {
return left;
- } else if (left == null && right != null) {
+ } else if (right.isEligible()) {
return right;
+ } else {
+ return new ExprNodeGenericFuncDescContext();
}
- return null;
}
case OR: {
- ExprNodeGenericFuncDesc left = convertToHivePartitionExpr(
- compoundPredicate.getChild(0), partitions, tblName);
- ExprNodeGenericFuncDesc right = convertToHivePartitionExpr(
+ ExprNodeGenericFuncDescContext left = convertToHivePartitionExpr(
compoundPredicate.getChild(0), partitions, tblName);
- if (left != null && right != null) {
- List<ExprNodeDesc> orArgs = new ArrayList<>();
- orArgs.add(left);
- orArgs.add(right);
- return getCompoundExpr(orArgs, "or");
- } else if (left != null && right == null) {
- return left;
- } else if (left == null && right != null) {
- return right;
+ ExprNodeGenericFuncDescContext right = convertToHivePartitionExpr(
+ compoundPredicate.getChild(1), partitions, tblName);
+ if (left.isEligible() && right.isEligible()) {
+ List<ExprNodeDesc> andArgs = new ArrayList<>();
+ andArgs.add(left.getFuncDesc());
+ andArgs.add(right.getFuncDesc());
+ return new ExprNodeGenericFuncDescContext(getCompoundExpr(andArgs, "or"));
+ } else {
+ // If it is not a partition key, this is an always true expr.
+ // Or if is a partition key and also is a not supportedOp, this is an always true expr.
+ return new ExprNodeGenericFuncDescContext(genAlwaysTrueExpr(tblName));
Review Comment:
I think it can just return `new ExprNodeGenericFuncDescContext()` ?
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/HiveMetaStoreClientHelper.java:
##########
@@ -400,61 +401,123 @@ public static List<FieldSchema> getSchema(String dbName, String tableName, Strin
/**
* Convert Doris expr to Hive expr, only for partition column
- * @param dorisExpr
- * @param partitions
* @param tblName
* @return
* @throws DdlException
* @throws SemanticException
*/
- public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(Expr dorisExpr,
+ public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(List<Expr> conjuncts,
+ List<String> partitionKeys, String tblName) throws DdlException {
+ List<ExprNodeDesc> hivePredicates = new ArrayList<>();
+
+ for (Expr conjunct : conjuncts) {
+ ExprNodeGenericFuncDesc hiveExpr = HiveMetaStoreClientHelper.convertToHivePartitionExpr(
+ conjunct, partitionKeys, tblName).getFuncDesc();
+ if (hiveExpr != null) {
+ hivePredicates.add(hiveExpr);
+ }
+ }
+ int count = hivePredicates.size();
+ // combine all predicate by `and`
+ // compoundExprs must have at least 2 predicates
+ if (count >= 2) {
+ return HiveMetaStoreClientHelper.getCompoundExpr(hivePredicates, "and");
+ } else if (count == 1) {
+ // only one predicate
+ return (ExprNodeGenericFuncDesc) hivePredicates.get(0);
+ } else {
+ return genAlwaysTrueExpr(tblName);
+ }
+ }
+
+ private static ExprNodeGenericFuncDesc genAlwaysTrueExpr(String tblName) throws DdlException {
+ // have no predicate, make a dummy predicate "1=1" to get all partitions
+ HiveMetaStoreClientHelper.ExprBuilder exprBuilder =
+ new HiveMetaStoreClientHelper.ExprBuilder(tblName);
+ return exprBuilder.val(TypeInfoFactory.intTypeInfo, 1)
+ .val(TypeInfoFactory.intTypeInfo, 1)
+ .pred("=", 2).build();
+ }
+
+ private static class ExprNodeGenericFuncDescContext {
+ private final ExprNodeGenericFuncDesc funcDesc;
+ private final boolean eligible;
+
+ public ExprNodeGenericFuncDescContext(ExprNodeGenericFuncDesc funcDesc) {
+ this.funcDesc = funcDesc;
+ this.eligible = true;
+ }
+
+ public ExprNodeGenericFuncDescContext() {
+ this.funcDesc = null;
+ this.eligible = false;
+ }
+
+ /**
+ * Check eligible before use the expr in CompoundPredicate for `and` and `or` .
+ */
+ public boolean isEligible() {
+ return eligible;
+ }
+
+ public ExprNodeGenericFuncDesc getFuncDesc() {
+ return funcDesc;
+ }
+ }
+
+ private static ExprNodeGenericFuncDescContext convertToHivePartitionExpr(Expr dorisExpr,
List<String> partitions, String tblName) throws DdlException {
Review Comment:
```suggestion
List<String> partitionKeys, String tblName) throws DdlException {
```
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/HiveMetaStoreClientHelper.java:
##########
@@ -400,61 +401,123 @@ public static List<FieldSchema> getSchema(String dbName, String tableName, Strin
/**
* Convert Doris expr to Hive expr, only for partition column
- * @param dorisExpr
- * @param partitions
* @param tblName
* @return
* @throws DdlException
* @throws SemanticException
*/
- public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(Expr dorisExpr,
+ public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(List<Expr> conjuncts,
+ List<String> partitionKeys, String tblName) throws DdlException {
+ List<ExprNodeDesc> hivePredicates = new ArrayList<>();
+
+ for (Expr conjunct : conjuncts) {
+ ExprNodeGenericFuncDesc hiveExpr = HiveMetaStoreClientHelper.convertToHivePartitionExpr(
+ conjunct, partitionKeys, tblName).getFuncDesc();
+ if (hiveExpr != null) {
+ hivePredicates.add(hiveExpr);
+ }
+ }
+ int count = hivePredicates.size();
+ // combine all predicate by `and`
+ // compoundExprs must have at least 2 predicates
+ if (count >= 2) {
+ return HiveMetaStoreClientHelper.getCompoundExpr(hivePredicates, "and");
+ } else if (count == 1) {
+ // only one predicate
+ return (ExprNodeGenericFuncDesc) hivePredicates.get(0);
+ } else {
+ return genAlwaysTrueExpr(tblName);
+ }
+ }
+
+ private static ExprNodeGenericFuncDesc genAlwaysTrueExpr(String tblName) throws DdlException {
+ // have no predicate, make a dummy predicate "1=1" to get all partitions
+ HiveMetaStoreClientHelper.ExprBuilder exprBuilder =
+ new HiveMetaStoreClientHelper.ExprBuilder(tblName);
+ return exprBuilder.val(TypeInfoFactory.intTypeInfo, 1)
+ .val(TypeInfoFactory.intTypeInfo, 1)
+ .pred("=", 2).build();
+ }
+
+ private static class ExprNodeGenericFuncDescContext {
+ private final ExprNodeGenericFuncDesc funcDesc;
+ private final boolean eligible;
+
+ public ExprNodeGenericFuncDescContext(ExprNodeGenericFuncDesc funcDesc) {
+ this.funcDesc = funcDesc;
+ this.eligible = true;
+ }
+
+ public ExprNodeGenericFuncDescContext() {
+ this.funcDesc = null;
+ this.eligible = false;
+ }
+
+ /**
+ * Check eligible before use the expr in CompoundPredicate for `and` and `or` .
+ */
+ public boolean isEligible() {
+ return eligible;
+ }
+
+ public ExprNodeGenericFuncDesc getFuncDesc() {
+ return funcDesc;
+ }
+ }
+
+ private static ExprNodeGenericFuncDescContext convertToHivePartitionExpr(Expr dorisExpr,
List<String> partitions, String tblName) throws DdlException {
if (dorisExpr == null) {
- return null;
+ return new ExprNodeGenericFuncDescContext();
}
if (dorisExpr instanceof CompoundPredicate) {
CompoundPredicate compoundPredicate = (CompoundPredicate) dorisExpr;
switch (compoundPredicate.getOp()) {
case AND: {
- ExprNodeGenericFuncDesc left = convertToHivePartitionExpr(
- compoundPredicate.getChild(0), partitions, tblName);
- ExprNodeGenericFuncDesc right = convertToHivePartitionExpr(
+ ExprNodeGenericFuncDescContext left = convertToHivePartitionExpr(
compoundPredicate.getChild(0), partitions, tblName);
- if (left != null && right != null) {
+ ExprNodeGenericFuncDescContext right = convertToHivePartitionExpr(
+ compoundPredicate.getChild(1), partitions, tblName);
+
+ if (left.isEligible() && right.isEligible()) {
List<ExprNodeDesc> andArgs = new ArrayList<>();
- andArgs.add(left);
- andArgs.add(right);
- return getCompoundExpr(andArgs, "and");
- } else if (left != null && right == null) {
+ andArgs.add(left.getFuncDesc());
+ andArgs.add(right.getFuncDesc());
+ return new ExprNodeGenericFuncDescContext(getCompoundExpr(andArgs, "and"));
+ } else if (left.isEligible()) {
return left;
- } else if (left == null && right != null) {
+ } else if (right.isEligible()) {
return right;
+ } else {
+ return new ExprNodeGenericFuncDescContext();
}
- return null;
}
case OR: {
- ExprNodeGenericFuncDesc left = convertToHivePartitionExpr(
- compoundPredicate.getChild(0), partitions, tblName);
- ExprNodeGenericFuncDesc right = convertToHivePartitionExpr(
+ ExprNodeGenericFuncDescContext left = convertToHivePartitionExpr(
Review Comment:
```
ExprNodeGenericFuncDescContext left = convertToHivePartitionExpr(
compoundPredicate.getChild(0), partitions, tblName);
ExprNodeGenericFuncDescContext right = convertToHivePartitionExpr(
compoundPredicate.getChild(1), partitions, tblName);
```
This part can be extracted out of `switch case` block
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #10547: [multi-catalog]Fix hive partiton prune in hive and hudi exteranl table.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10547:
URL: https://github.com/apache/doris/pull/10547#issuecomment-1174679407
PR approved by anyone and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman commented on pull request #10547: [External Table]Fix hive partiton prune in hive and hudi exteranl table.
Posted by GitBox <gi...@apache.org>.
morningman commented on PR #10547:
URL: https://github.com/apache/doris/pull/10547#issuecomment-1172828875
It is strange that all exprs should pass the `NormalizeBinaryPredicatesRule`, and expr like `1 < col` should be converted to `col > 1` already.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] SaintBacchus commented on pull request #10547: [External Table]Fix hive partiton prune in hive and hudi exteranl table.
Posted by GitBox <gi...@apache.org>.
SaintBacchus commented on PR #10547:
URL: https://github.com/apache/doris/pull/10547#issuecomment-1172831336
> It is strange that all exprs should pass the `NormalizeBinaryPredicatesRule`, and expr like `1 < col` should be converted to `col > 1` already.
it get the col,value and op expr one by one, and then compose they by fixed order. so it will cause issue
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman merged pull request #10547: [multi-catalog]Fix hive partiton prune in hive and hudi exteranl table.
Posted by GitBox <gi...@apache.org>.
morningman merged PR #10547:
URL: https://github.com/apache/doris/pull/10547
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman commented on a diff in pull request #10547: [External Table]Fix hive partiton prune in hive and hudi exteranl table.
Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #10547:
URL: https://github.com/apache/doris/pull/10547#discussion_r912795745
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/HiveMetaStoreClientHelper.java:
##########
@@ -465,68 +559,58 @@ public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(Expr dorisExpr,
case LT:
case EQ_FOR_NULL:
BinaryPredicate eq = (BinaryPredicate) dorisExpr;
+ // Make sure the col slot is always first
SlotRef slotRef = convertDorisExprToSlotRef(eq.getChild(0));
- LiteralExpr literalExpr = null;
- if (slotRef == null && eq.getChild(0).isLiteral()) {
- literalExpr = (LiteralExpr) eq.getChild(0);
- slotRef = convertDorisExprToSlotRef(eq.getChild(1));
- } else if (eq.getChild(1).isLiteral()) {
- literalExpr = (LiteralExpr) eq.getChild(1);
- }
+ LiteralExpr literalExpr = (LiteralExpr) eq.getChild(1);
Review Comment:
The `eq.getChild(1)` may NOT be a `LiteralExpr`, this may throw CastException.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/HiveMetaStoreClientHelper.java:
##########
@@ -400,61 +401,154 @@ public static List<FieldSchema> getSchema(String dbName, String tableName, Strin
/**
* Convert Doris expr to Hive expr, only for partition column
- * @param dorisExpr
- * @param partitions
* @param tblName
* @return
* @throws DdlException
* @throws SemanticException
*/
- public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(Expr dorisExpr,
+ public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(List<Expr> conjuncts,
+ List<String> partitionKeys, String tblName) throws DdlException {
+ List<ExprNodeDesc> hivePredicates = new ArrayList<>();
+
+ for (Expr conjunct : conjuncts) {
+ ExprNodeGenericFuncDesc hiveExpr = HiveMetaStoreClientHelper.convertToHivePartitionExpr(
+ conjunct, partitionKeys, tblName).getFuncDesc();
+ if (hiveExpr != null) {
+ hivePredicates.add(hiveExpr);
+ }
+ }
+ int count = hivePredicates.size();
+ // combine all predicate by `and`
+ // compoundExprs must have at least 2 predicates
+ if (count >= 2) {
+ return HiveMetaStoreClientHelper.getCompoundExpr(hivePredicates, "and");
+ } else if (count == 1) {
+ // only one predicate
+ return (ExprNodeGenericFuncDesc) hivePredicates.get(0);
+ } else {
+ return genAlwaysTrueExpr(tblName);
+ }
+ }
+
+ private static ExprNodeGenericFuncDesc genAlwaysTrueExpr(String tblName) throws DdlException {
+ // have no predicate, make a dummy predicate "1=1" to get all partitions
+ HiveMetaStoreClientHelper.ExprBuilder exprBuilder =
+ new HiveMetaStoreClientHelper.ExprBuilder(tblName);
+ return exprBuilder.val(TypeInfoFactory.intTypeInfo, 1)
+ .val(TypeInfoFactory.intTypeInfo, 1)
+ .pred("=", 2).build();
+ }
+
+ private static class ExprNodeGenericFuncDescContext {
+ private final ExprNodeGenericFuncDesc funcDesc;
+ private final boolean isPartitionFiled;
+ private final boolean supportedOp;
+
+ public ExprNodeGenericFuncDescContext(ExprNodeGenericFuncDesc funcDesc) {
+ this.funcDesc = funcDesc;
+ this.isPartitionFiled = true;
+ this.supportedOp = true;
+ }
+
+ public ExprNodeGenericFuncDescContext(boolean isPartitionFiled, boolean supportedOp) {
+ this.funcDesc = null;
+ this.isPartitionFiled = true;
Review Comment:
unused input parameters?
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/HiveMetaStoreClientHelper.java:
##########
@@ -400,61 +401,154 @@ public static List<FieldSchema> getSchema(String dbName, String tableName, Strin
/**
* Convert Doris expr to Hive expr, only for partition column
- * @param dorisExpr
- * @param partitions
* @param tblName
* @return
* @throws DdlException
* @throws SemanticException
*/
- public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(Expr dorisExpr,
+ public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(List<Expr> conjuncts,
+ List<String> partitionKeys, String tblName) throws DdlException {
+ List<ExprNodeDesc> hivePredicates = new ArrayList<>();
+
+ for (Expr conjunct : conjuncts) {
+ ExprNodeGenericFuncDesc hiveExpr = HiveMetaStoreClientHelper.convertToHivePartitionExpr(
+ conjunct, partitionKeys, tblName).getFuncDesc();
+ if (hiveExpr != null) {
+ hivePredicates.add(hiveExpr);
+ }
+ }
+ int count = hivePredicates.size();
+ // combine all predicate by `and`
+ // compoundExprs must have at least 2 predicates
+ if (count >= 2) {
+ return HiveMetaStoreClientHelper.getCompoundExpr(hivePredicates, "and");
+ } else if (count == 1) {
+ // only one predicate
+ return (ExprNodeGenericFuncDesc) hivePredicates.get(0);
+ } else {
+ return genAlwaysTrueExpr(tblName);
+ }
+ }
+
+ private static ExprNodeGenericFuncDesc genAlwaysTrueExpr(String tblName) throws DdlException {
+ // have no predicate, make a dummy predicate "1=1" to get all partitions
+ HiveMetaStoreClientHelper.ExprBuilder exprBuilder =
+ new HiveMetaStoreClientHelper.ExprBuilder(tblName);
+ return exprBuilder.val(TypeInfoFactory.intTypeInfo, 1)
+ .val(TypeInfoFactory.intTypeInfo, 1)
+ .pred("=", 2).build();
+ }
+
+ private static class ExprNodeGenericFuncDescContext {
+ private final ExprNodeGenericFuncDesc funcDesc;
+ private final boolean isPartitionFiled;
Review Comment:
```suggestion
private final boolean isPartitionField;
```
?
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/HiveMetaStoreClientHelper.java:
##########
@@ -400,61 +401,154 @@ public static List<FieldSchema> getSchema(String dbName, String tableName, Strin
/**
* Convert Doris expr to Hive expr, only for partition column
- * @param dorisExpr
- * @param partitions
* @param tblName
* @return
* @throws DdlException
* @throws SemanticException
*/
- public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(Expr dorisExpr,
+ public static ExprNodeGenericFuncDesc convertToHivePartitionExpr(List<Expr> conjuncts,
+ List<String> partitionKeys, String tblName) throws DdlException {
+ List<ExprNodeDesc> hivePredicates = new ArrayList<>();
+
+ for (Expr conjunct : conjuncts) {
+ ExprNodeGenericFuncDesc hiveExpr = HiveMetaStoreClientHelper.convertToHivePartitionExpr(
+ conjunct, partitionKeys, tblName).getFuncDesc();
+ if (hiveExpr != null) {
+ hivePredicates.add(hiveExpr);
+ }
+ }
+ int count = hivePredicates.size();
+ // combine all predicate by `and`
+ // compoundExprs must have at least 2 predicates
+ if (count >= 2) {
+ return HiveMetaStoreClientHelper.getCompoundExpr(hivePredicates, "and");
+ } else if (count == 1) {
+ // only one predicate
+ return (ExprNodeGenericFuncDesc) hivePredicates.get(0);
+ } else {
+ return genAlwaysTrueExpr(tblName);
+ }
+ }
+
+ private static ExprNodeGenericFuncDesc genAlwaysTrueExpr(String tblName) throws DdlException {
+ // have no predicate, make a dummy predicate "1=1" to get all partitions
+ HiveMetaStoreClientHelper.ExprBuilder exprBuilder =
+ new HiveMetaStoreClientHelper.ExprBuilder(tblName);
+ return exprBuilder.val(TypeInfoFactory.intTypeInfo, 1)
+ .val(TypeInfoFactory.intTypeInfo, 1)
+ .pred("=", 2).build();
+ }
+
+ private static class ExprNodeGenericFuncDescContext {
+ private final ExprNodeGenericFuncDesc funcDesc;
+ private final boolean isPartitionFiled;
+ private final boolean supportedOp;
+
+ public ExprNodeGenericFuncDescContext(ExprNodeGenericFuncDesc funcDesc) {
+ this.funcDesc = funcDesc;
+ this.isPartitionFiled = true;
+ this.supportedOp = true;
+ }
+
+ public ExprNodeGenericFuncDescContext(boolean isPartitionFiled, boolean supportedOp) {
+ this.funcDesc = null;
+ this.isPartitionFiled = true;
+ this.supportedOp = true;
+ }
+
+ /**
+ * If it's a partition key and also is a supportedOp, we can go through into next loop.
+ */
+ public boolean keepForAndOp() {
+ return isPartitionFiled && supportedOp;
+ }
+
+ /**
+ * If it's not a partition key, we should ignore this expr in `or`.
+ */
+ public boolean isPartitionFiled() {
+ return isPartitionFiled;
+ }
+
+ /**
+ * And if is a partition key and also is a not supportedOp, this is an always true expr.
+ */
+ public boolean alwaysTrueForOrOp() {
+ return isPartitionFiled && !supportedOp;
+ }
+
+ public ExprNodeGenericFuncDesc getFuncDesc() {
+ return funcDesc;
+ }
+ }
+
+ private static ExprNodeGenericFuncDescContext convertToHivePartitionExpr(Expr dorisExpr,
List<String> partitions, String tblName) throws DdlException {
if (dorisExpr == null) {
- return null;
+ return new ExprNodeGenericFuncDescContext(false, false);
}
if (dorisExpr instanceof CompoundPredicate) {
CompoundPredicate compoundPredicate = (CompoundPredicate) dorisExpr;
switch (compoundPredicate.getOp()) {
case AND: {
- ExprNodeGenericFuncDesc left = convertToHivePartitionExpr(
+ ExprNodeGenericFuncDescContext left = convertToHivePartitionExpr(
compoundPredicate.getChild(0), partitions, tblName);
- ExprNodeGenericFuncDesc right = convertToHivePartitionExpr(
- compoundPredicate.getChild(0), partitions, tblName);
- if (left != null && right != null) {
+ ExprNodeGenericFuncDescContext right = convertToHivePartitionExpr(
+ compoundPredicate.getChild(1), partitions, tblName);
+
+ if (left.keepForAndOp() && right.keepForAndOp()) {
List<ExprNodeDesc> andArgs = new ArrayList<>();
- andArgs.add(left);
- andArgs.add(right);
- return getCompoundExpr(andArgs, "and");
- } else if (left != null && right == null) {
+ andArgs.add(left.getFuncDesc());
+ andArgs.add(right.getFuncDesc());
+ return new ExprNodeGenericFuncDescContext(getCompoundExpr(andArgs, "and"));
+ } else if (left.keepForAndOp()) {
return left;
- } else if (left == null && right != null) {
+ } else if (right.keepForAndOp()) {
return right;
+ } else {
+ return new ExprNodeGenericFuncDescContext(false, false);
}
- return null;
}
case OR: {
- ExprNodeGenericFuncDesc left = convertToHivePartitionExpr(
- compoundPredicate.getChild(0), partitions, tblName);
- ExprNodeGenericFuncDesc right = convertToHivePartitionExpr(
+ ExprNodeGenericFuncDescContext left = convertToHivePartitionExpr(
compoundPredicate.getChild(0), partitions, tblName);
- if (left != null && right != null) {
- List<ExprNodeDesc> orArgs = new ArrayList<>();
- orArgs.add(left);
- orArgs.add(right);
- return getCompoundExpr(orArgs, "or");
- } else if (left != null && right == null) {
- return left;
- } else if (left == null && right != null) {
- return right;
+ ExprNodeGenericFuncDescContext right = convertToHivePartitionExpr(
+ compoundPredicate.getChild(1), partitions, tblName);
+ // no matter left or right can drop, we must drop both for this example:
+ // partition_key1 = 1 or partition_key2 in (2,3) which `in` is a nonsupport op now.
+ if (left.alwaysTrueForOrOp() || right.alwaysTrueForOrOp()) {
+ // return 1 = 1 expr for these examples:
+ // (partition_key1 = 1 or partition_key2 in (2,3)) or partition_key3 = 4 => always true
+ // (partition_key1 = 1 or partition_key2 in (2,3)) and partition_key3 = 4 => partition_key3 = 4
+ return new ExprNodeGenericFuncDescContext(genAlwaysTrueExpr(tblName));
+ } else {
+ if (left.isPartitionFiled() && right.isPartitionFiled()) {
+ List<ExprNodeDesc> orArgs = new ArrayList<>();
+ orArgs.add(left.getFuncDesc());
+ orArgs.add(right.getFuncDesc());
+ return new ExprNodeGenericFuncDescContext(getCompoundExpr(orArgs, "or"));
+ } else if (left.isPartitionFiled()) {
Review Comment:
I think we should return `alwaysTrue` is only one side is partition field?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #10547: [multi-catalog]Fix hive partiton prune in hive and hudi exteranl table.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10547:
URL: https://github.com/apache/doris/pull/10547#issuecomment-1174679388
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org