[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3772: [CARBONDATA-3832]Added block and blocket pruning for the polygon expression processing
VenuReddy2103 commented on a change in pull request #3772: URL: https://github.com/apache/carbondata/pull/3772#discussion_r445481119 ## File path: core/src/main/java/org/apache/carbondata/core/scan/filter/FilterUtil.java ## @@ -188,6 +189,14 @@ private static FilterExecuter createFilterExecuterTree( return new FalseFilterExecutor(); case ROWLEVEL: default: + if (filterExpressionResolverTree.getFilterExpression() instanceof UnknownExpression) { +FilterExecuter filterExecuter = +((UnknownExpression) filterExpressionResolverTree.getFilterExpression()) +.getFilterExecuter(filterExpressionResolverTree, segmentProperties); +if (filterExecuter != null) { Review comment: I understand what you meant. I think, it is better to create the RowLevelFilterExecuterImpl only at this place. If UnknownExpression do not have any specific implemenation, it will be like a fallback to RowLevelFilterExecuterImpl. Also, if we have return RowLevelFilterExecuterImpl from UnknownExpression.getFilterExecuter, we need to pass complexDimensionInfoMap and all overiding methods will have this argument. It is an unused argument as current UnKnown expressions do not support complex dimensions. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3772: [CARBONDATA-3832]Added block and blocket pruning for the polygon expression processing
VenuReddy2103 commented on a change in pull request #3772: URL: https://github.com/apache/carbondata/pull/3772#discussion_r445475371 ## File path: integration/spark/src/test/scala/org/apache/carbondata/geo/GeoTest.scala ## @@ -134,8 +133,31 @@ class GeoTest extends QueryTest with BeforeAndAfterAll with BeforeAndAfterEach { Row(116285807, 40084087))) } - test("test polygon query on table partitioned by timevalue column") - { + test("test block pruning for polygon query") { +createTable() +sql(s"insert into $table1 select 157542840,116285807,40084087") +sql(s"insert into $table1 select 157542840,116372142,40129503") +sql(s"insert into $table1 select 157542840,116187332,39979316") +sql(s"insert into $table1 select 157542840,116337069,39951887") +sql(s"insert into $table1 select 157542840,116359102,40154684") +sql(s"insert into $table1 select 157542840,116736367,39970323") +sql(s"insert into $table1 select 157542840,116362699,39942444") +sql(s"insert into $table1 select 157542840,116325378,39963129") +sql(s"insert into $table1 select 157542840,116302895,39930753") +sql(s"insert into $table1 select 157542840,116288955,3101") +val df = sql(s"select longitude, latitude from $table1 where IN_POLYGON('116.321011 " + + s"40.123503, 116.137676 39.947911, 116.560993 39.935276, 116.321011 40.123503')") +assert(df.rdd.getNumPartitions == 6) +checkAnswer(df, Review comment: Ok. Instead of making the Seq of rows with same rows in all testcases, have defined it once and used same in all the testcases. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3772: [CARBONDATA-3832]Added block and blocket pruning for the polygon expression processing
VenuReddy2103 commented on a change in pull request #3772: URL: https://github.com/apache/carbondata/pull/3772#discussion_r440394383 ## File path: geo/src/main/java/org/apache/carbondata/geo/scan/expression/PolygonExpression.java ## @@ -46,15 +51,16 @@ private CustomIndex> instance; private List ranges = new ArrayList(); private ColumnExpression column; - private ExpressionResult trueExpRes; - private ExpressionResult falseExpRes; + private static final ExpressionResult trueExpRes = Review comment: Have replied in the immediate below comment. Please let me know if you have any other opinion. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3772: [CARBONDATA-3832]Added block and blocket pruning for the polygon expression processing
VenuReddy2103 commented on a change in pull request #3772: URL: https://github.com/apache/carbondata/pull/3772#discussion_r440386464 ## File path: core/src/main/java/org/apache/carbondata/core/scan/filter/FilterUtil.java ## @@ -188,6 +189,14 @@ private static FilterExecuter createFilterExecuterTree( return new FalseFilterExecutor(); case ROWLEVEL: default: + if (filterExpressionResolverTree.getFilterExpression() instanceof UnknownExpression) { Review comment: In the initial version of the feature PR, polygon expression and executor type were defined explitily for polygon filter in carbondata-core module itself. But Ravi's review comment suggested the use of `UnknownExpression`(similar to `SparkUnknownExpression`) and have carbondata-geo depends on carbondata-core. But not the other way round. Currently `UnknownExpression` always uses `RowLevelFilterExecuterImpl` and applies filter for each row without pruning(i.e., no block, blocklet and page pruning). So have enhanced the `UnknownExpression` with pruning ability. ## File path: geo/src/main/java/org/apache/carbondata/geo/scan/expression/PolygonExpression.java ## @@ -46,15 +51,16 @@ private CustomIndex> instance; private List ranges = new ArrayList(); private ColumnExpression column; - private ExpressionResult trueExpRes; - private ExpressionResult falseExpRes; + private static final ExpressionResult trueExpRes = Review comment: Have replied for it the immediate below comment. Please let me know if you have any other opinion. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3772: [CARBONDATA-3832]Added block and blocket pruning for the polygon expression processing
VenuReddy2103 commented on a change in pull request #3772: URL: https://github.com/apache/carbondata/pull/3772#discussion_r440386464 ## File path: core/src/main/java/org/apache/carbondata/core/scan/filter/FilterUtil.java ## @@ -188,6 +189,14 @@ private static FilterExecuter createFilterExecuterTree( return new FalseFilterExecutor(); case ROWLEVEL: default: + if (filterExpressionResolverTree.getFilterExpression() instanceof UnknownExpression) { Review comment: In the initial version of the feature PR, polygon expression and executor type were defined explitily for polygon filter in carbondata-core module itself. But Ravi's review comment suggested the use of `UnknownExpression`(similar to `SparkUnknownExpression`) and have carbondata-geo depends on carbondata-core. But not the other way round. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3772: [CARBONDATA-3832]Added block and blocket pruning for the polygon expression processing
VenuReddy2103 commented on a change in pull request #3772: URL: https://github.com/apache/carbondata/pull/3772#discussion_r440378220 ## File path: geo/src/main/java/org/apache/carbondata/geo/scan/filter/executor/PolygonFilterExecutorImpl.java ## @@ -0,0 +1,84 @@ +package org.apache.carbondata.geo.scan.filter.executor; + +import java.util.BitSet; +import java.util.List; +import java.util.Map; + +import org.apache.carbondata.common.logging.LogServiceFactory; +import org.apache.carbondata.core.datastore.block.SegmentProperties; +import org.apache.carbondata.core.datastore.chunk.impl.DimensionRawColumnChunk; +import org.apache.carbondata.core.metadata.AbsoluteTableIdentifier; +import org.apache.carbondata.core.metadata.datatype.DataTypes; +import org.apache.carbondata.core.scan.expression.Expression; +import org.apache.carbondata.core.scan.filter.GenericQueryType; +import org.apache.carbondata.core.scan.filter.executer.RowLevelFilterExecuterImpl; +import org.apache.carbondata.core.scan.filter.resolver.resolverinfo.DimColumnResolvedFilterInfo; +import org.apache.carbondata.core.scan.filter.resolver.resolverinfo.MeasureColumnResolvedFilterInfo; +import org.apache.carbondata.core.scan.processor.RawBlockletColumnChunks; +import org.apache.carbondata.core.util.DataTypeUtil; +import org.apache.carbondata.geo.scan.expression.PolygonExpression; + +import org.apache.log4j.Logger; + +public class PolygonFilterExecutorImpl extends RowLevelFilterExecuterImpl { + public PolygonFilterExecutorImpl(List dimColEvaluatorInfoList, + List msrColEvalutorInfoList, Expression exp, + AbsoluteTableIdentifier tableIdentifier, SegmentProperties segmentProperties, + Map complexDimensionInfoMap) { +super(dimColEvaluatorInfoList, msrColEvalutorInfoList, exp, tableIdentifier, segmentProperties, +complexDimensionInfoMap); + } + + private int getNearestRangeIndex(List ranges, long searchForNumber) { +Long[] range; +int low = 0, mid = 0, high = ranges.size() - 1; +while (low <= high) { + mid = low + ((high - low) / 2); + range = ranges.get(mid); + if (searchForNumber >= range[0]) { +if (searchForNumber <= range[1]) { + // Return the range index if the number is between min and max values of the range + return mid; +} else { + // Number is bigger than this range's min and max. Search on the right side of the range + low = mid + 1; +} + } else { +// Number is smaller than this range's min and max. Search on the left side of the range +high = mid - 1; + } +} +return mid; + } + + private boolean isScanRequired(byte[] maxValue, byte[] minValue) { Review comment: Added method header and comments to code 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3772: [CARBONDATA-3832]Added block and blocket pruning for the polygon expression processing
VenuReddy2103 commented on a change in pull request #3772: URL: https://github.com/apache/carbondata/pull/3772#discussion_r440377390 ## File path: geo/src/main/java/org/apache/carbondata/geo/scan/filter/executor/PolygonFilterExecutorImpl.java ## @@ -0,0 +1,84 @@ +package org.apache.carbondata.geo.scan.filter.executor; Review comment: Added 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] VenuReddy2103 commented on a change in pull request #3772: [CARBONDATA-3832]Added block and blocket pruning for the polygon expression processing
VenuReddy2103 commented on a change in pull request #3772: URL: https://github.com/apache/carbondata/pull/3772#discussion_r440377512 ## File path: geo/src/main/java/org/apache/carbondata/geo/scan/filter/executor/PolygonFilterExecutorImpl.java ## @@ -0,0 +1,84 @@ +package org.apache.carbondata.geo.scan.filter.executor; + +import java.util.BitSet; +import java.util.List; +import java.util.Map; + +import org.apache.carbondata.common.logging.LogServiceFactory; +import org.apache.carbondata.core.datastore.block.SegmentProperties; +import org.apache.carbondata.core.datastore.chunk.impl.DimensionRawColumnChunk; +import org.apache.carbondata.core.metadata.AbsoluteTableIdentifier; +import org.apache.carbondata.core.metadata.datatype.DataTypes; +import org.apache.carbondata.core.scan.expression.Expression; +import org.apache.carbondata.core.scan.filter.GenericQueryType; +import org.apache.carbondata.core.scan.filter.executer.RowLevelFilterExecuterImpl; +import org.apache.carbondata.core.scan.filter.resolver.resolverinfo.DimColumnResolvedFilterInfo; +import org.apache.carbondata.core.scan.filter.resolver.resolverinfo.MeasureColumnResolvedFilterInfo; +import org.apache.carbondata.core.scan.processor.RawBlockletColumnChunks; +import org.apache.carbondata.core.util.DataTypeUtil; +import org.apache.carbondata.geo.scan.expression.PolygonExpression; + +import org.apache.log4j.Logger; + +public class PolygonFilterExecutorImpl extends RowLevelFilterExecuterImpl { + public PolygonFilterExecutorImpl(List dimColEvaluatorInfoList, Review comment: done 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org