[GitHub] [carbondata] kunal642 commented on a change in pull request #3885: [CARBONDATA-3946] Support IndexServer with Presto Engine

2020-08-17 Thread GitBox 


kunal642 commented on a change in pull request #3885:
URL: https://github.com/apache/carbondata/pull/3885#discussion_r471901716



##
File path: 
integration/presto/src/main/prestodb/org/apache/carbondata/presto/impl/CarbonTableReader.java
##
@@ -281,7 +287,11 @@ private CarbonTableCacheModel 
getValidCacheBySchemaTableName(SchemaTableName sch
   createInputFormat(jobConf, carbonTable.getAbsoluteTableIdentifier(),
   new IndexFilter(carbonTable, filters, true), filteredPartitions);
   Job job = Job.getInstance(jobConf);
+  CarbonProperties.getInstance()
+  .addProperty(CarbonCommonConstants.IS_QUERY_FROM_PRESTO, "true");

Review comment:
   @ajantha-bhat If we have a common place, better to move there





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] kunal642 commented on a change in pull request #3885: [CARBONDATA-3946] Support IndexServer with Presto Engine

2020-08-12 Thread GitBox 


kunal642 commented on a change in pull request #3885:
URL: https://github.com/apache/carbondata/pull/3885#discussion_r469700182



##
File path: 
integration/presto/src/main/prestodb/org/apache/carbondata/presto/impl/CarbonTableReader.java
##
@@ -281,7 +287,11 @@ private CarbonTableCacheModel 
getValidCacheBySchemaTableName(SchemaTableName sch
   createInputFormat(jobConf, carbonTable.getAbsoluteTableIdentifier(),
   new IndexFilter(carbonTable, filters, true), filteredPartitions);
   Job job = Job.getInstance(jobConf);
+  CarbonProperties.getInstance()
+  .addProperty(CarbonCommonConstants.IS_QUERY_FROM_PRESTO, "true");

Review comment:
   Can we move this initialization to a more common class..currently this 
will be set only in case of read





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] kunal642 commented on a change in pull request #3885: [CARBONDATA-3946] Support IndexServer with Presto Engine

2020-08-12 Thread GitBox 


kunal642 commented on a change in pull request #3885:
URL: https://github.com/apache/carbondata/pull/3885#discussion_r469287811



##
File path: 
integration/spark/src/main/scala/org/apache/carbondata/indexserver/IndexJobs.scala
##
@@ -55,14 +57,27 @@ class DistributedIndexJob extends AbstractIndexJob {
 val (response, time) = logTime {
   try {
 val spark = SparkSQLUtil.getSparkSession
-indexFormat.setTaskGroupId(SparkSQLUtil.getTaskGroupId(spark))
-indexFormat.setTaskGroupDesc(SparkSQLUtil.getTaskGroupDesc(spark))
+// In case of presto with index server flow, sparksession will be null
+if (null != spark) {

Review comment:
   is it possible to add a configuration to CarbonProperties that can tell 
us whether its a presto flow or not. Property should not be exposed to user. 
just for internal purpose.
   
   This null check looks dirty.





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