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-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
