niuge01 commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r400679356 ########## File path: docs/index-developer-guide.md ########## @@ -24,11 +24,11 @@ Currently, there are two types of DataMap supported: 2. MVDataMap: DataMap that leverages Materialized View to accelerate olap style query, like SPJG query (select, predicate, join, groupby). Preaggregate, timeseries and mv DataMap belong to this type of DataMaps. Review comment: Yes, we will do it in a new pr. ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
niuge01 commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r400681680 ########## File path: integration/spark/src/test/scala/org/apache/carbondata/view/MaterializedViewTest.scala ########## @@ -0,0 +1,194 @@ +package org.apache.carbondata.view + +import java.io.File + +import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException +import org.apache.spark.sql.catalyst.catalog.HiveTableRelation +import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan +import org.apache.spark.sql.execution.datasources.LogicalRelation +import org.apache.spark.sql.test.util.QueryTest +import org.scalatest.BeforeAndAfterAll + +import org.apache.carbondata.core.constants.CarbonCommonConstants +import org.apache.carbondata.core.util.CarbonProperties +import org.apache.carbondata.spark.exception.ProcessMetaDataException + +class MaterializedViewTest extends QueryTest with BeforeAndAfterAll { + + override def beforeAll { + drop() + CarbonProperties.getInstance() + .addProperty(CarbonCommonConstants.CARBON_TIMESTAMP_FORMAT, "yyyy/MM/dd") + val projectPath = new File(this.getClass.getResource("/").getPath + "../../../../") + .getCanonicalPath.replaceAll("\\\\", "/") + val integrationPath = s"$projectPath/integration" + val resourcesPath = s"$integrationPath/spark/src/test/resources" + sql( + """ + | CREATE TABLE fact_table (empname String, designation String, doj Timestamp, + | workgroupcategory int, workgroupcategoryname String, deptno int, deptname String, + | projectcode int, projectjoindate Timestamp, projectenddate Timestamp,attendance int, + | utilization int,salary int) + | STORED AS carbondata + """.stripMargin) + sql(s"""LOAD DATA local inpath '$resourcesPath/data_big.csv' INTO TABLE fact_table OPTIONS('DELIMITER'= ',', 'QUOTECHAR'= '"')""") + sql(s"""LOAD DATA local inpath '$resourcesPath/data_big.csv' INTO TABLE fact_table OPTIONS('DELIMITER'= ',', 'QUOTECHAR'= '"')""") + } + + test("test create mv on hive table") { + sql("drop materialized view if exists mv1") + sql("drop table if exists source") + sql("create table source as select * from fact_table") + sql("create materialized view mv1 as select empname, deptname, avg(salary) from source group by empname, deptname") Review comment: Currently, all mv tables which created by CREATE MATERIALIZED VIEW statement is carbon mv table. ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
niuge01 commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r400681805 ########## File path: core/src/main/java/org/apache/carbondata/core/datamap/IndexChooser.java ########## @@ -59,76 +59,76 @@ * the datamap which has fewer columns that is the first datamap. */ @InterfaceAudience.Internal -public class DataMapChooser { +public class IndexChooser { private CarbonTable carbonTable; - private List<TableDataMap> cgDataMaps; - private List<TableDataMap> fgDataMaps; + private List<TableIndex> cgIndexes; + private List<TableIndex> fgIndexes; - public DataMapChooser(CarbonTable carbonTable) throws IOException { + public IndexChooser(CarbonTable carbonTable) throws IOException { this.carbonTable = carbonTable; - // read all datamaps for this table and populate CG and FG datamap list - List<TableDataMap> visibleDataMaps = - DataMapStoreManager.getInstance().getAllVisibleDataMap(carbonTable); + // read all indexes for this table and populate CG and FG index list + List<TableIndex> visibleIndexes = + DataMapStoreManager.getInstance().getAllVisibleIndexes(carbonTable); Map<String, DataMapStatusDetail> map = DataMapStatusManager.readDataMapStatusMap(); - cgDataMaps = new ArrayList<>(visibleDataMaps.size()); - fgDataMaps = new ArrayList<>(visibleDataMaps.size()); - for (TableDataMap visibleDataMap : visibleDataMaps) { - DataMapStatusDetail status = map.get(visibleDataMap.getDataMapSchema().getDataMapName()); + cgIndexes = new ArrayList<>(visibleIndexes.size()); + fgIndexes = new ArrayList<>(visibleIndexes.size()); + for (TableIndex visibleIndex : visibleIndexes) { + DataMapStatusDetail status = map.get(visibleIndex.getDataMapSchema().getDataMapName()); if (status != null && status.isEnabled()) { - DataMapLevel level = visibleDataMap.getDataMapFactory().getDataMapLevel(); - if (level == DataMapLevel.CG) { - cgDataMaps.add(visibleDataMap); + IndexLevel level = visibleIndex.getIndexFactory().getDataMapLevel(); + if (level == IndexLevel.CG) { + cgIndexes.add(visibleIndex); } else { - fgDataMaps.add(visibleDataMap); + fgIndexes.add(visibleIndex); } } } } /** - * Return a chosen datamap based on input filter. See {@link DataMapChooser} + * Return a chosen datamap based on input filter. See {@link IndexChooser} */ - public DataMapExprWrapper choose(FilterResolverIntf filter) { + public IndexExprWrapper choose(FilterResolverIntf filter) { if (filter != null) { Expression expression = filter.getFilterExpression(); // First check for FG datamaps if any exist - ExpressionTuple tuple = selectDataMap(expression, fgDataMaps, filter); - if (tuple.dataMapExprWrapper == null) { + ExpressionTuple tuple = selectDataMap(expression, fgIndexes, filter); + if (tuple.indexExprWrapper == null) { // Check for CG datamap - tuple = selectDataMap(expression, cgDataMaps, filter); + tuple = selectDataMap(expression, cgIndexes, filter); } - if (tuple.dataMapExprWrapper != null) { - return tuple.dataMapExprWrapper; + if (tuple.indexExprWrapper != null) { + return tuple.indexExprWrapper; } } // Return the default datamap if no other datamap exists. - return new DataMapExprWrapperImpl( - DataMapStoreManager.getInstance().getDefaultDataMap(carbonTable), filter); + return new IndexExprWrapperImpl( + DataMapStoreManager.getInstance().getDefaultIndex(carbonTable), filter); } /** - * Return a chosen FG datamap based on input filter. See {@link DataMapChooser} + * Return a chosen FG datamap based on input filter. See {@link IndexChooser} */ - public DataMapExprWrapper chooseFGDataMap(FilterResolverIntf resolverIntf) { - return chooseDataMap(DataMapLevel.FG, resolverIntf); + public IndexExprWrapper chooseFGDataMap(FilterResolverIntf resolverIntf) { + return chooseDataMap(IndexLevel.FG, resolverIntf); } /** - * Return a chosen CG datamap based on input filter. See {@link DataMapChooser} + * Return a chosen CG datamap based on input filter. See {@link IndexChooser} */ - public DataMapExprWrapper chooseCGDataMap(FilterResolverIntf resolverIntf) { - return chooseDataMap(DataMapLevel.CG, resolverIntf); + public IndexExprWrapper chooseCGDataMap(FilterResolverIntf resolverIntf) { + return chooseDataMap(IndexLevel.CG, resolverIntf); } - DataMapExprWrapper chooseDataMap(DataMapLevel level, FilterResolverIntf resolverIntf) { + IndexExprWrapper chooseDataMap(IndexLevel level, FilterResolverIntf resolverIntf) { if (resolverIntf != null) { Expression expression = resolverIntf.getFilterExpression(); - List<TableDataMap> datamaps = level == DataMapLevel.CG ? cgDataMaps : fgDataMaps; + List<TableIndex> datamaps = level == IndexLevel.CG ? cgIndexes : fgIndexes; Review comment: Yes, we will do it in a new pr. ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401312862 ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/CarbonExtensions.scala ########## @@ -76,6 +66,11 @@ case class CarbonOptimizerRule(session: SparkSession) extends Rule[LogicalPlan] self.synchronized { if (notAdded) { notAdded = false + + session.udf.register(MaterializedViewUDF.DUMMY_FUNCTION, () => "") Review comment: move code to CarbonEnv.init ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401303776 ########## File path: integration/spark/src/main/scala/org/apache/carbondata/events/DropTableEvents.scala ########## @@ -30,7 +30,8 @@ import org.apache.carbondata.core.metadata.schema.table.CarbonTable case class DropTablePreEvent( carbonTable: CarbonTable, ifExistsSet: Boolean, - sparkSession: SparkSession) + sparkSession: SparkSession, + isInternalCall: Boolean = false) Review comment: what does 'isInternalCall' mean? ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r395427793 ########## File path: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ########## @@ -460,7 +460,7 @@ private CarbonCommonConstants() { public static final String LOCAL_DICTIONARY_EXCLUDE = "local_dictionary_exclude"; /** - * DMPROPERTY for Index DataMap, like lucene, bloomfilter DataMap, + * DMPROPERTY for Index Index, like lucene, bloomfilter Index, Review comment: remove repeated Index ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401311390 ########## File path: integration/spark/src/main/scala/org/apache/carbondata/view/MaterializedViewManagerInSpark.scala ########## @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.carbondata.view + +import java.util + +import org.apache.spark.sql.{CarbonUtils, SparkSession} + +import org.apache.carbondata.core.constants.CarbonCommonConstants +import org.apache.carbondata.core.view.MaterializedViewManager + +class MaterializedViewManagerInSpark(session: SparkSession) extends MaterializedViewManager { + override def getDatabases: util.List[String] = { + CarbonUtils.threadSet(CarbonCommonConstants.DISABLE_SQL_REWRITE, "true") + try { + val databaseList = session.catalog.listDatabases() + val databaseNameList = new util.ArrayList[String]() + for (database <- databaseList.collect()) { + databaseNameList.add(database.name) + } + databaseNameList + } finally { + CarbonUtils.threadUnset(CarbonCommonConstants.DISABLE_SQL_REWRITE) + } + } +} + +object MaterializedViewManagerInSpark { + + private val MANAGER_MAP_BY_SESSION = + new util.HashMap[SparkSession, MaterializedViewManagerInSpark]() Review comment: please clean the spark session when it closed. check SparkSqlAdapter.addSparkSessionListener, should do same with CarbonEnv.carbonEnvMap ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401311666 ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/CarbonEnv.scala ########## @@ -160,29 +161,29 @@ object CarbonEnv { def initListeners(): Unit = { OperationListenerBus.getInstance() .addListener(classOf[IndexServerLoadEvent], PrePrimingEventListener) - .addListener(classOf[LoadTablePreExecutionEvent], LoadMVTablePreListener) - .addListener(classOf[AlterTableCompactionPreStatusUpdateEvent], - AlterDataMaptableCompactionPostListener) +// .addListener(classOf[LoadTablePreExecutionEvent], LoadMVTablePreListener) Review comment: if code is not needed, remove it. ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401314214 ########## File path: core/src/main/java/org/apache/carbondata/core/datamap/AbstractIndexJob.java ########## @@ -28,15 +28,15 @@ /** * abstract class for data map job Review comment: Change the comment content. If we change 'datamap' in the file name into 'index', better to change package name at same time. ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401314484 ########## File path: core/src/main/java/org/apache/carbondata/core/datamap/DataMapProvider.java ########## @@ -48,20 +48,20 @@ import org.apache.log4j.Logger; /** - * DataMap is a accelerator for certain type of query. Developer can add new DataMap + * Index is a accelerator for certain type of query. Developer can add new Index * implementation to improve query performance. * - * Currently two types of DataMap are supported + * Currently two types of Index are supported * <ol> - * <li> MVDataMap: materialized view type of DataMap to accelerate olap style query, + * <li> MVDataMap: materialized view type of Index to accelerate olap style query, * like SPJG query (select, predicate, join, groupby) </li> - * <li> DataMap: index type of DataMap to accelerate filter query </li> + * <li> Index: index type of Index to accelerate filter query </li> * </ol> * * <p> * In following command <br> * {@code CREATE DATAMAP dm ON TABLE main USING 'provider'}, <br> Review comment: do we still have this SQL syntax? ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401315843 ########## File path: core/src/main/java/org/apache/carbondata/core/datamap/IndexChooser.java ########## @@ -269,14 +269,14 @@ private void extractColumnExpression(Expression expression, } } - private TableDataMap chooseDataMap(List<TableDataMap> allDataMap, + private TableIndex chooseDataMap(List<TableIndex> allDataMap, Review comment: better to change all DataMap to index once. if not, code will be hard to 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401318776 ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/parser/CarbonSpark2SqlParser.scala ########## @@ -76,21 +77,19 @@ class CarbonSpark2SqlParser extends CarbonDDLSqlParser { } - protected lazy val start: Parser[LogicalPlan] = startCommand | extendedSparkSyntax + protected lazy val start: Parser[LogicalPlan] = + startCommand | extendedSparkSyntax | materializedViewCommands Review comment: append materializedViewCommands to startCommand ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401320625 ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/parser/CarbonSpark2SqlParser.scala ########## @@ -101,6 +100,12 @@ class CarbonSpark2SqlParser extends CarbonDDLSqlParser { loadDataNew | explainPlan | alterTableColumnRenameAndModifyDataType | alterTableAddColumns + protected lazy val materializedViewCommands: Parser[LogicalPlan] = Review comment: agree with @Indhumathi27, MVParser(MV module) should Keep independent from CarbonParser(integeration/spark module). ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401341097 ########## File path: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/CarbonTable.java ########## @@ -885,6 +894,16 @@ public boolean isChildTableForMV() { .get(CarbonCommonConstants.PARENT_TABLES).isEmpty(); } + /** + * Return true if this table is a MV table (child table of other table) + */ + public boolean isMaterializedView() { + return tableInfo.getFactTable().getTableProperties() + .get(CarbonCommonConstants.ASSOCIATED_TABLES) != null && + !tableInfo.getFactTable().getTableProperties() + .get(CarbonCommonConstants.ASSOCIATED_TABLES).isEmpty(); Review comment: please remove redundancy code. how about use MV_ON_TABLES OR MV_RELATED_TABLES instead of ASSOCIATED_TABLES ? ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401316964 ########## File path: core/src/main/java/org/apache/carbondata/core/datamap/status/DataMapStatus.java ########## @@ -18,7 +18,7 @@ package org.apache.carbondata.core.datamap.status; /** - * DataMap status + * Index status */ public enum DataMapStatus { Review comment: strongly agree with @ajantha-bhat ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401339656 ########## File path: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/CarbonTable.java ########## @@ -876,6 +876,15 @@ public boolean hasMVCreated() throws IOException { schema.getProviderName().equalsIgnoreCase(DataMapClassProvider.MV.toString())); } + /** + * Return true if this table is a MV table (child table of other table) + */ + public boolean isMVTable() { + String parentTables = tableInfo.getFactTable().getTableProperties() + .get(CarbonCommonConstants.PARENT_TABLES); Review comment: How about changing it to MV_ON_TABlES? What is the difference with method isChildTableForMV/isMaterializedView ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401320051 ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/parser/CarbonSpark2SqlParser.scala ########## @@ -101,6 +100,12 @@ class CarbonSpark2SqlParser extends CarbonDDLSqlParser { loadDataNew | explainPlan | alterTableColumnRenameAndModifyDataType | alterTableAddColumns + protected lazy val materializedViewCommands: Parser[LogicalPlan] = Review comment: At some places, we use MaterializedView, but at some other places, we use MV. What's the rule? Better to unify to use "MV" word in code(beside of SQL syntax) ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401323108 ########## File path: mv/plan/src/main/scala/org/apache/carbondata/mv/plans/util/BirdcageOptimizer.scala ########## @@ -124,8 +123,9 @@ object BirdcageOptimizer extends RuleExecutor[LogicalPlan] { // Batch("LocalRelation", fixedPoint, // ConvertToLocalRelation, // PropagateEmptyRelation) :: - Batch( - "OptimizeCodegen", Once, CarbonToSparkAdapter.getOptimizeCodegenRule(): _*) :: + // As per SPARK-22520 OptimizeCodegen is removed in 2.3.1 Review comment: remove it if not required ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401342546 ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/hive/execution/command/CarbonHiveCommands.scala ########## @@ -30,7 +30,7 @@ import org.apache.carbondata.common.logging.LogServiceFactory import org.apache.carbondata.core.constants.{CarbonCommonConstants, CarbonCommonConstantsInternal, CarbonLoadOptionConstants} import org.apache.carbondata.core.datamap.DataMapStoreManager import org.apache.carbondata.core.exception.InvalidConfigurationException -import org.apache.carbondata.core.util.{BlockletDataMapUtil, CarbonProperties, CarbonUtil, SessionParams} +import org.apache.carbondata.core.util.{BlockletIndexUtil, CarbonProperties, CarbonUtil, SessionParams} Review comment: remove unused import ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on a change in pull request #3661: [CARBONDATA-3704] Support create materialized view on all type table, and make mv support mutil-tenant.
URL: https://github.com/apache/carbondata/pull/3661#discussion_r401316305 ########## File path: core/src/main/java/org/apache/carbondata/core/datamap/IndexMeta.java ########## @@ -29,24 +29,24 @@ import org.apache.commons.lang3.StringUtils; /** - * Metadata of the datamap, set by DataMap developer + * Metadata of the datamap, set by Index developer */ -@InterfaceAudience.Developer("DataMap") +@InterfaceAudience.Developer("Index") @InterfaceStability.Evolving -public class DataMapMeta { +public class IndexMeta { Review comment: if we change the class name, better to change the content also. ---------------------------------------------------------------- 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: [hidden email] With regards, Apache Git Services |
Free forum by Nabble | Edit this page |