[GitHub] carbondata pull request #2334: [CARBONDATA-2515][CARBONDATA-2516] fixed Time...
Github user asfgit closed the pull request at: https://github.com/apache/carbondata/pull/2334 ---
[GitHub] carbondata pull request #2334: [CARBONDATA-2515][CARBONDATA-2516] fixed Time...
Github user sv71294 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2334#discussion_r197722635
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java
---
@@ -171,134 +170,89 @@ else if
(colType.equals(DecimalType.createDecimalType(carbondataColumnHandle.get
* @return
*/
static Expression parseFilterExpression(TupleDomain
originalConstraint) {
-ImmutableList.Builder filters = ImmutableList.builder();
Domain domain;
+Expression finalFilters = null;
--- End diff --
sure, adding comments in code
---
[GitHub] carbondata pull request #2334: [CARBONDATA-2515][CARBONDATA-2516] fixed Time...
Github user chenliang613 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2334#discussion_r197721852
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java
---
@@ -171,134 +170,89 @@ else if
(colType.equals(DecimalType.createDecimalType(carbondataColumnHandle.get
* @return
*/
static Expression parseFilterExpression(TupleDomain
originalConstraint) {
-ImmutableList.Builder filters = ImmutableList.builder();
Domain domain;
+Expression finalFilters = null;
--- End diff --
1. I mean, please put these detail explanation inside the code in your pr
2. colExpression, just column's name and data type, why may change ?
---
[GitHub] carbondata pull request #2334: [CARBONDATA-2515][CARBONDATA-2516] fixed Time...
Github user sv71294 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2334#discussion_r197678803
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java
---
@@ -171,134 +170,89 @@ else if
(colType.equals(DecimalType.createDecimalType(carbondataColumnHandle.get
* @return
*/
static Expression parseFilterExpression(TupleDomain
originalConstraint) {
-ImmutableList.Builder filters = ImmutableList.builder();
Domain domain;
+Expression finalFilters = null;
--- End diff --
1. finalFilters => this one is the final expression for the table, this
variable is returned by the method after combining all the column filters
(colValueExpression).
2. colExpression => we have used final to make sure that value of this
variable doesn't get change during the execution of iteration for respective
column.
3. colValueExpression => this is the combination of multiple
rangeExpression for a single column in case of multiple filters on single
column else this is equal to rangeExpression
4.rangeExpression => this is generated for each range of column i.e.
lessThan, greaterThan, there can be multiple ranges for a single column.
---
[GitHub] carbondata pull request #2334: [CARBONDATA-2515][CARBONDATA-2516] fixed Time...
Github user xubo245 commented on a diff in the pull request: https://github.com/apache/carbondata/pull/2334#discussion_r190103982 --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java --- @@ -92,14 +69,15 @@ private static DataType Spi2CarbondataTypeMapper(CarbondataColumnHandle carbonda else if (colType == DateType.DATE) return DataTypes.DATE; else if (colType == TimestampType.TIMESTAMP) return DataTypes.TIMESTAMP; else if (colType.equals(DecimalType.createDecimalType(carbondataColumnHandle.getPrecision(), -carbondataColumnHandle.getScale( return DataTypes -.createDecimalType(carbondataColumnHandle.getPrecision(), -carbondataColumnHandle.getScale()); +carbondataColumnHandle.getScale( return DataTypes --- End diff -- Why change this one? ---
[GitHub] carbondata pull request #2334: [CARBONDATA-2515][CARBONDATA-2516] fixed Time...
Github user xubo245 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2334#discussion_r190104064
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java
---
@@ -110,28 +88,30 @@ else if
(colType.equals(DecimalType.createDecimalType(carbondataColumnHandle.get
for (ColumnHandle columnHandle :
originalConstraint.getDomains().get().keySet()) {
CarbondataColumnHandle carbondataColumnHandle =
(CarbondataColumnHandle) columnHandle;
List partitionedColumnSchema =
columnSchemas.stream().filter(
- columnSchema ->
carbondataColumnHandle.getColumnName().equals(columnSchema.getColumnName())).collect(toList());
- if(partitionedColumnSchema.size() != 0) {
+ columnSchema ->
carbondataColumnHandle.getColumnName().equals(columnSchema.getColumnName())).collect(toList());
+ if (partitionedColumnSchema.size() != 0) {
filter.addAll(createPartitionFilters(originalConstraint,
carbondataColumnHandle));
}
}
return filter;
}
- /** Returns list of partition key and values using domain constraints
+ /**
+ * Returns list of partition key and values using domain constraints
+ *
* @param originalConstraint
* @param carbonDataColumnHandle
*/
private static List
createPartitionFilters(TupleDomain originalConstraint,
- CarbondataColumnHandle carbonDataColumnHandle) {
+
CarbondataColumnHandle carbonDataColumnHandle) {
--- End diff --
Why change this one?
---
[GitHub] carbondata pull request #2334: [CARBONDATA-2515][CARBONDATA-2516] fixed Time...
Github user xubo245 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2334#discussion_r190104019
--- Diff:
integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java
---
@@ -110,28 +88,30 @@ else if
(colType.equals(DecimalType.createDecimalType(carbondataColumnHandle.get
for (ColumnHandle columnHandle :
originalConstraint.getDomains().get().keySet()) {
CarbondataColumnHandle carbondataColumnHandle =
(CarbondataColumnHandle) columnHandle;
List partitionedColumnSchema =
columnSchemas.stream().filter(
- columnSchema ->
carbondataColumnHandle.getColumnName().equals(columnSchema.getColumnName())).collect(toList());
- if(partitionedColumnSchema.size() != 0) {
+ columnSchema ->
carbondataColumnHandle.getColumnName().equals(columnSchema.getColumnName())).collect(toList());
--- End diff --
Why change this one?
---
[GitHub] carbondata pull request #2334: [CARBONDATA-2515][CARBONDATA-2516] fixed Time...
Github user xubo245 commented on a diff in the pull request: https://github.com/apache/carbondata/pull/2334#discussion_r190103900 --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java --- @@ -17,51 +17,28 @@ package org.apache.carbondata.presto; -import java.math.BigDecimal; -import java.math.BigInteger; -import java.sql.Timestamp; -import java.util.ArrayList; -import java.util.Calendar; -import java.util.Date; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - +import com.facebook.presto.spi.ColumnHandle; +import com.facebook.presto.spi.PrestoException; +import com.facebook.presto.spi.predicate.Domain; +import com.facebook.presto.spi.predicate.Range; +import com.facebook.presto.spi.predicate.TupleDomain; +import com.facebook.presto.spi.type.*; +import io.airlift.slice.Slice; import org.apache.carbondata.core.metadata.datatype.DataType; import org.apache.carbondata.core.metadata.datatype.DataTypes; import org.apache.carbondata.core.metadata.schema.table.CarbonTable; import org.apache.carbondata.core.metadata.schema.table.column.ColumnSchema; import org.apache.carbondata.core.scan.expression.ColumnExpression; import org.apache.carbondata.core.scan.expression.Expression; import org.apache.carbondata.core.scan.expression.LiteralExpression; -import org.apache.carbondata.core.scan.expression.conditional.EqualToExpression; -import org.apache.carbondata.core.scan.expression.conditional.GreaterThanEqualToExpression; -import org.apache.carbondata.core.scan.expression.conditional.GreaterThanExpression; -import org.apache.carbondata.core.scan.expression.conditional.InExpression; -import org.apache.carbondata.core.scan.expression.conditional.LessThanEqualToExpression; -import org.apache.carbondata.core.scan.expression.conditional.LessThanExpression; -import org.apache.carbondata.core.scan.expression.conditional.ListExpression; +import org.apache.carbondata.core.scan.expression.conditional.*; import org.apache.carbondata.core.scan.expression.logical.AndExpression; import org.apache.carbondata.core.scan.expression.logical.OrExpression; -import com.facebook.presto.spi.ColumnHandle; -import com.facebook.presto.spi.PrestoException; -import com.facebook.presto.spi.predicate.Domain; -import com.facebook.presto.spi.predicate.Range; -import com.facebook.presto.spi.predicate.TupleDomain; -import com.facebook.presto.spi.type.BigintType; -import com.facebook.presto.spi.type.BooleanType; -import com.facebook.presto.spi.type.DateType; -import com.facebook.presto.spi.type.DecimalType; -import com.facebook.presto.spi.type.Decimals; -import com.facebook.presto.spi.type.DoubleType; -import com.facebook.presto.spi.type.IntegerType; -import com.facebook.presto.spi.type.SmallintType; -import com.facebook.presto.spi.type.TimestampType; -import com.facebook.presto.spi.type.Type; -import com.facebook.presto.spi.type.VarcharType; -import com.google.common.collect.ImmutableList; -import io.airlift.slice.Slice; +import java.math.BigDecimal; --- End diff -- please optimize the order of class, unify with others ---
[GitHub] carbondata pull request #2334: [CARBONDATA-2515][CARBONDATA-2516] fixed Time...
Github user xubo245 commented on a diff in the pull request: https://github.com/apache/carbondata/pull/2334#discussion_r190103764 --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java --- @@ -17,51 +17,28 @@ package org.apache.carbondata.presto; -import java.math.BigDecimal; -import java.math.BigInteger; -import java.sql.Timestamp; -import java.util.ArrayList; -import java.util.Calendar; -import java.util.Date; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - +import com.facebook.presto.spi.ColumnHandle; +import com.facebook.presto.spi.PrestoException; +import com.facebook.presto.spi.predicate.Domain; +import com.facebook.presto.spi.predicate.Range; +import com.facebook.presto.spi.predicate.TupleDomain; +import com.facebook.presto.spi.type.*; +import io.airlift.slice.Slice; import org.apache.carbondata.core.metadata.datatype.DataType; import org.apache.carbondata.core.metadata.datatype.DataTypes; import org.apache.carbondata.core.metadata.schema.table.CarbonTable; import org.apache.carbondata.core.metadata.schema.table.column.ColumnSchema; import org.apache.carbondata.core.scan.expression.ColumnExpression; import org.apache.carbondata.core.scan.expression.Expression; import org.apache.carbondata.core.scan.expression.LiteralExpression; -import org.apache.carbondata.core.scan.expression.conditional.EqualToExpression; -import org.apache.carbondata.core.scan.expression.conditional.GreaterThanEqualToExpression; -import org.apache.carbondata.core.scan.expression.conditional.GreaterThanExpression; -import org.apache.carbondata.core.scan.expression.conditional.InExpression; -import org.apache.carbondata.core.scan.expression.conditional.LessThanEqualToExpression; -import org.apache.carbondata.core.scan.expression.conditional.LessThanExpression; -import org.apache.carbondata.core.scan.expression.conditional.ListExpression; +import org.apache.carbondata.core.scan.expression.conditional.*; import org.apache.carbondata.core.scan.expression.logical.AndExpression; import org.apache.carbondata.core.scan.expression.logical.OrExpression; -import com.facebook.presto.spi.ColumnHandle; -import com.facebook.presto.spi.PrestoException; -import com.facebook.presto.spi.predicate.Domain; -import com.facebook.presto.spi.predicate.Range; -import com.facebook.presto.spi.predicate.TupleDomain; -import com.facebook.presto.spi.type.BigintType; -import com.facebook.presto.spi.type.BooleanType; -import com.facebook.presto.spi.type.DateType; -import com.facebook.presto.spi.type.DecimalType; -import com.facebook.presto.spi.type.Decimals; -import com.facebook.presto.spi.type.DoubleType; -import com.facebook.presto.spi.type.IntegerType; -import com.facebook.presto.spi.type.SmallintType; -import com.facebook.presto.spi.type.TimestampType; -import com.facebook.presto.spi.type.Type; -import com.facebook.presto.spi.type.VarcharType; -import com.google.common.collect.ImmutableList; -import io.airlift.slice.Slice; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.sql.Timestamp; +import java.util.*; --- End diff -- Please keep the detail class, not * ---
