[GitHub] carbondata pull request #2148: [CARBONDATA-2323][WIP] Distributed search mod...

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2148
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4997/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2148
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4442/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2148
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5011/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2148
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3795/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2148
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5024/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2148
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3808/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2148
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3809/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2148
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5025/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2148
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5027/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2148
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3811/



---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2148#discussion_r181585854
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/datatype/DataType.java ---
    @@ -91,6 +91,7 @@ public static char convertType(DataType dataType) {
           return STRING_CHAR;
         } else if (dataType == DataTypes.TIMESTAMP) {
           return TIMESTAMP_CHAR;
    +
    --- End diff --
   
    remove unnecessary line


---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2148#discussion_r181586050
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/executor/impl/SearchModeVectorDetailQueryExecutor.java ---
    @@ -56,6 +59,11 @@
         }
       }
     
    +  public static void shutdown() {
    --- End diff --
   
    I don't find the caller method of this method.


---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2148#discussion_r181586088
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/executor/impl/SearchModeVectorDetailQueryExecutor.java ---
    @@ -56,6 +59,11 @@
         }
       }
     
    +  public static void shutdown() {
    --- End diff --
   
    Better override the finish method to call the executorservice shutdowm


---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2148#discussion_r181586769
 
    --- Diff: integration/spark2/src/main/java/org/apache/carbondata/spark/vectorreader/VectorizedCarbonRecordReader.java ---
    @@ -134,9 +133,7 @@ public void initialize(InputSplit inputSplit, TaskAttemptContext taskAttemptCont
         queryModel.setTableBlockInfos(tableBlockInfoList);
         queryModel.setVectorReader(true);
         try {
    -      if (CarbonProperties.getInstance().getProperty(
    -              CarbonCommonConstants.CARBON_SEARCH_MODE_ENABLE,
    -              CarbonCommonConstants.CARBON_SEARCH_MODE_ENABLE_DEFAULT).equals("true")) {
    +      if (CarbonProperties.isSearchModeEnabled()) {
    --- End diff --
   
    I guess no need to do if check here, already `QueryExecutorFactory` available so move the code there.


---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2148#discussion_r181587317
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/datatype/DataType.java ---
    @@ -91,6 +91,7 @@ public static char convertType(DataType dataType) {
           return STRING_CHAR;
         } else if (dataType == DataTypes.TIMESTAMP) {
           return TIMESTAMP_CHAR;
    +
    --- End diff --
   
    fixed


---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2148#discussion_r181587500
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/executor/impl/SearchModeVectorDetailQueryExecutor.java ---
    @@ -56,6 +59,11 @@
         }
       }
     
    +  public static void shutdown() {
    --- End diff --
   
    This is called in Worker.shutdown


---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2148#discussion_r181587533
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/model/QueryModelBuilder.java ---
    @@ -23,51 +23,101 @@
     import org.apache.carbondata.core.metadata.schema.table.column.CarbonDimension;
     import org.apache.carbondata.core.metadata.schema.table.column.CarbonMeasure;
     import org.apache.carbondata.core.scan.expression.Expression;
    +import org.apache.carbondata.core.scan.filter.SingleTableProvider;
     import org.apache.carbondata.core.scan.filter.resolver.FilterResolverIntf;
    +import org.apache.carbondata.core.util.DataTypeConverter;
     
     public class QueryModelBuilder {
     
    -  private CarbonTable carbonTable;
    +  private CarbonTable table;
    +  private QueryProjection projection;
    +  private Expression filterExpression;
    +  private DataTypeConverter dataTypeConverter;
    +  private boolean forcedDetailRawQuery;
    +  private boolean readPageByPage;
     
    -  public QueryModelBuilder(CarbonTable carbonTable) {
    -    this.carbonTable = carbonTable;
    +  public QueryModelBuilder(CarbonTable table) {
    +    this.table = table;
       }
     
    -  public QueryModel build(String[] projectionColumnNames, Expression filterExpression) {
    -    QueryModel queryModel = QueryModel.newInstance(carbonTable);
    -    QueryProjection projection = carbonTable.createProjection(projectionColumnNames);
    -    queryModel.setProjection(projection);
    -    boolean[] isFilterDimensions = new boolean[carbonTable.getDimensionOrdinalMax()];
    -    boolean[] isFilterMeasures = new boolean[carbonTable.getAllMeasures().size()];
    -    carbonTable.processFilterExpression(filterExpression, isFilterDimensions, isFilterMeasures);
    -    queryModel.setIsFilterDimensions(isFilterDimensions);
    -    queryModel.setIsFilterMeasures(isFilterMeasures);
    -    FilterResolverIntf filterIntf = carbonTable.resolveFilter(filterExpression, null);
    -    queryModel.setFilterExpressionResolverTree(filterIntf);
    -    return queryModel;
    +  public QueryModelBuilder projectColumns(String[] projectionColumns) {
    +    String factTableName = table.getTableName();
    +    QueryProjection projection = new QueryProjection();
    +    // fill dimensions
    +    // If columns are null, set all dimensions and measures
    --- End diff --
   
    I think it is better to throw an exception if projections are null. Because it has already another method `projectAllColumns` if the user wants to get all columns.


---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2148#discussion_r181587611
 
    --- Diff: integration/spark2/src/main/java/org/apache/carbondata/spark/vectorreader/VectorizedCarbonRecordReader.java ---
    @@ -134,9 +133,7 @@ public void initialize(InputSplit inputSplit, TaskAttemptContext taskAttemptCont
         queryModel.setTableBlockInfos(tableBlockInfoList);
         queryModel.setVectorReader(true);
         try {
    -      if (CarbonProperties.getInstance().getProperty(
    -              CarbonCommonConstants.CARBON_SEARCH_MODE_ENABLE,
    -              CarbonCommonConstants.CARBON_SEARCH_MODE_ENABLE_DEFAULT).equals("true")) {
    +      if (CarbonProperties.isSearchModeEnabled()) {
    --- End diff --
   
    fixed


---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2148#discussion_r181587662
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/model/QueryModelBuilder.java ---
    @@ -23,51 +23,101 @@
     import org.apache.carbondata.core.metadata.schema.table.column.CarbonDimension;
     import org.apache.carbondata.core.metadata.schema.table.column.CarbonMeasure;
     import org.apache.carbondata.core.scan.expression.Expression;
    +import org.apache.carbondata.core.scan.filter.SingleTableProvider;
     import org.apache.carbondata.core.scan.filter.resolver.FilterResolverIntf;
    +import org.apache.carbondata.core.util.DataTypeConverter;
     
     public class QueryModelBuilder {
     
    -  private CarbonTable carbonTable;
    +  private CarbonTable table;
    +  private QueryProjection projection;
    +  private Expression filterExpression;
    +  private DataTypeConverter dataTypeConverter;
    +  private boolean forcedDetailRawQuery;
    +  private boolean readPageByPage;
     
    -  public QueryModelBuilder(CarbonTable carbonTable) {
    -    this.carbonTable = carbonTable;
    +  public QueryModelBuilder(CarbonTable table) {
    +    this.table = table;
       }
     
    -  public QueryModel build(String[] projectionColumnNames, Expression filterExpression) {
    -    QueryModel queryModel = QueryModel.newInstance(carbonTable);
    -    QueryProjection projection = carbonTable.createProjection(projectionColumnNames);
    -    queryModel.setProjection(projection);
    -    boolean[] isFilterDimensions = new boolean[carbonTable.getDimensionOrdinalMax()];
    -    boolean[] isFilterMeasures = new boolean[carbonTable.getAllMeasures().size()];
    -    carbonTable.processFilterExpression(filterExpression, isFilterDimensions, isFilterMeasures);
    -    queryModel.setIsFilterDimensions(isFilterDimensions);
    -    queryModel.setIsFilterMeasures(isFilterMeasures);
    -    FilterResolverIntf filterIntf = carbonTable.resolveFilter(filterExpression, null);
    -    queryModel.setFilterExpressionResolverTree(filterIntf);
    -    return queryModel;
    +  public QueryModelBuilder projectColumns(String[] projectionColumns) {
    +    String factTableName = table.getTableName();
    +    QueryProjection projection = new QueryProjection();
    +    // fill dimensions
    +    // If columns are null, set all dimensions and measures
    --- End diff --
   
    fixed


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2148
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5028/



---