[GitHub] carbondata pull request #1888: [CARBONDATA-2101]Restrict direct query on pre...

[qiuchenjian-2]

GitHub user kumarvishal09 opened a pull request:

    https://github.com/apache/carbondata/pull/1888

    [CARBONDATA-2101]Restrict direct query on pre aggregate and timeseries datamap

    **Restricting direct query on PreAggregate and timeseries data map
    Added Property to run direct query on data map for testing purpose
    validate.support.direct.query.on.datamap=true**
   
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
   
     - [ ] Testing done
            Added UT Test case
           
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
   


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/kumarvishal09/incubator-carbondata Restrict_DirectQuery_OnDatamap

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/carbondata/pull/1888.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1888
   
----
commit b5aaf49f289394f3800fa5e8b3133ba79ef0d869
Author: Manohar <manohar.crazy09@...>
Date:   2017-12-20T09:39:45Z

    Restrict direct query on pre aggregate and timeseries datamap

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1888#discussion_r164933498
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -1567,6 +1567,14 @@
     
       public static final String CARBON_ENABLE_PAGE_LEVEL_READER_IN_COMPACTION_DEFAULT = "true";
     
    +  @CarbonProperty
    +  public static final String SUPPORT_DIRECT_QUERY_ON_DATAMAP =
    +      "carbon.support.direct.query.on.datamap";
    --- End diff --
   
    provide default value for these two newly added property


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1888#discussion_r164933909
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/optimizer/CarbonLateDecodeRule.scala ---
    @@ -87,6 +88,39 @@ class CarbonLateDecodeRule extends Rule[LogicalPlan] with PredicateHelper {
         }
       }
     
    +  /**
    +   * Below method will be used to validate if query is directly fired on pre aggregate
    +   * data map or not
    +   * @param relations all relations from query
    +   *
    +   */
    +  def validateQueryDirectlyOnDataMap(relations: Seq[CarbonDecoderRelation]): Unit = {
    +    var isPreAggDataMapExists = false
    +    // first check if pre aggregate data map exists or not
    +    relations.foreach{relation =>
    +      if (relation.carbonRelation.carbonTable.isChildDataMap) {
    +        isPreAggDataMapExists = true
    +      }
    +    }
    +    val validateQuery = CarbonProperties.getInstance
    +      .getProperty(CarbonCommonConstants.VALIDATE_DIRECT_QUERY_ON_DATAMAP, "true").toBoolean
    +    var isThrowException = false
    +    // if validate query is enabled and relation contains pre aggregate data map
    +    if (validateQuery && isPreAggDataMapExists) {
    +      val carbonSessionInfo = ThreadLocalSessionInfo.getCarbonSessionInfo
    --- End diff --
   
    Can you expose a put and get interface in CarbonEnv to manipulate the SessionParam in CarbonEnv


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1888#discussion_r164962280
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonPreAggregateRules.scala ---
    @@ -461,6 +467,8 @@ case class CarbonPreAggregateQueryRules(sparkSession: SparkSession) extends Rule
                       childPlan,
                       carbonTable,
                       agg)
    +              CarbonSession.threadSet(CarbonCommonConstants.SUPPORT_DIRECT_QUERY_ON_DATAMAP,
    --- End diff --
   
    Can you avoid calling it multiple times , instead call only once


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1888#discussion_r165013843
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -1567,6 +1567,14 @@
     
       public static final String CARBON_ENABLE_PAGE_LEVEL_READER_IN_COMPACTION_DEFAULT = "true";
     
    +  @CarbonProperty
    +  public static final String SUPPORT_DIRECT_QUERY_ON_DATAMAP =
    +      "carbon.support.direct.query.on.datamap";
    --- End diff --
   
    Fixed


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user kumarvishal09 commented on the issue:

    https://github.com/apache/carbondata/pull/1888
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1888
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/2166/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user kumarvishal09 commented on the issue:

    https://github.com/apache/carbondata/pull/1888
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1888
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/2175/



---