[CARBONDATA-4138] Reordering Carbon Expression instead of Spark Filter #4100
Conversation
QiangCai
changed the title
|
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/3348/ |
|
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/5107/ |
QiangCai
force-pushed
the
expression_reorder
branch
from
3b27966 to
6899941
Compare
QiangCai
force-pushed
the
expression_reorder
branch
from
6899941 to
0eb8b36
Compare
|
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/5110/ |
|
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/3351/ |
|
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/5112/ |
|
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/3353/ |
|
retest this please |
|
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/5118/ |
|
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/3359/ |
|
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3764/ |
|
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5529/ |
|
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/3291/ |
|
retest this please |
|
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/5049/ |
|
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5542/ |
|
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3775/ |
|
retest this please |
|
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5549/ |
|
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3783/ |
|
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/5061/ |
|
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3309/ |
|
retest this please |
|
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3342/ |
|
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5094/ |
|
@kunal642 please review and merge |
| @@ -2602,6 +2602,11 @@ private CarbonCommonConstants() { | |||
|
|
|||
| public static final String FILE_HEADER = "fileHeader"; | |||
|
|
|||
| @CarbonProperty(dynamicConfigurable = true) | |||
| public static final String CARBON_OPTIMIZE_FILTER = "carbon.optimize.filter"; | |||
There was a problem hiding this comment.
"carbon.reorder.filter" and "carbon.optimize.filter" seem to be doing the same thing. please remove one of them
There was a problem hiding this comment.
reorder is one part of optimaztion
| */ | ||
| public class ExpressionOptimizer { | ||
|
|
||
| private final OptimizeRule[] rules = { new ExpressionReorder() }; |
There was a problem hiding this comment.
no need for private static class, make this final and use directly
| } | ||
| MultiExpression multiExpression = MultiExpression.build(expression); | ||
| // unsupported expression | ||
| if (multiExpression == null) { |
There was a problem hiding this comment.
move this check above MultiExpression.build so that we dont enter the reorder code if null
There was a problem hiding this comment.
MultiExpression.build return "multiExpression "
| /** | ||
| * a wrapper class of Expression with storage ordinal | ||
| */ | ||
| public class ExpressionWithOrdinal extends StorageOrdinal { |
There was a problem hiding this comment.
all classes inside optimize package except ExpressionReorder should have default access modifier so that they are not used for creating expressions by mistake
| import org.apache.carbondata.core.scan.expression.Expression; | ||
|
|
||
| /** | ||
| * |
There was a problem hiding this comment.
remove the empty description
| } | ||
|
|
||
| test("Test filter reorder with various conditions") { | ||
| checkOptimizer("(four = 11 and two = 11) or (one = 11)", |
There was a problem hiding this comment.
why pass the filter expression as string instead of Expression object?? No need for translation.
base on pr#4097
Why is this PR needed?
reorder Carbon Expression instead of Spark Filter
What changes were proposed in this PR?
reorder Carbon Expression instead of Spark Filter
Does this PR introduce any user interface change?
Is any new testcase added?