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_r401322301 ########## File path: mv/plan/src/main/scala/org/apache/carbondata/mv/plans/modular/Harmonizer.scala ########## @@ -72,8 +69,8 @@ object HarmonizeDimensionTable extends Rule[ModularPlan] with PredicateHelper { case s@Select(_, _, _, _, jedges, fact :: dims, _, _, _, _) if jedges.forall(e => e.joinType == LeftOuter || e.joinType == Inner) && fact.isInstanceOf[ModularRelation] && - dims.filterNot(_.isInstanceOf[modular.LeafNode]).nonEmpty && - dims.forall(d => (d.isInstanceOf[ModularRelation] || HarmonizedRelation.canHarmonize(d))) => { + !dims.forall(_.isInstanceOf[modular.LeafNode]) && Review comment: dims.exists(!_.isInstanceOf[modular.LeafNode] ---------------------------------------------------------------- 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_r401325282 ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/command/SIRebuildSegmentRunner.scala ########## @@ -35,32 +34,24 @@ import org.apache.carbondata.common.exceptions.sql.MalformedCarbonCommandExcepti import org.apache.carbondata.common.logging.LogServiceFactory import org.apache.carbondata.core.locks.{CarbonLockFactory, LockUsage} import org.apache.carbondata.core.metadata.{CarbonTableIdentifier, ColumnarFormatVersion} -import org.apache.carbondata.core.metadata.schema.table.{CarbonTable, TableInfo} +import org.apache.carbondata.core.metadata.schema.table.CarbonTable import org.apache.carbondata.core.statusmanager.{LoadMetadataDetails, SegmentStatus, SegmentStatusManager} import org.apache.carbondata.core.util.CarbonUtil import org.apache.carbondata.events.{OperationContext, OperationListenerBus} -case class SIRebuildSegmentCommand( - alterTableModel: AlterTableModel, - tableInfoOp: Option[TableInfo] = None, - operationContext: OperationContext = new OperationContext) - extends AtomicRunnableCommand { +case class SIRebuildSegmentRunner( Review comment: how about SIRebuildSegmentExecutor? ---------------------------------------------------------------- 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_r401336799 ########## File path: core/src/main/java/org/apache/carbondata/core/view/MaterializedViewCatalog.java ########## @@ -0,0 +1,47 @@ +/* Review comment: how about move core/src/main/java/org/apache/carbondata/core/view to mv-plan 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
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_r401355336 ########## 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: OK ---------------------------------------------------------------- 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_r401355488 ########## 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: 'isInternalCall' is mean trigger by other command, not by user directly. ---------------------------------------------------------------- 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_r401355696 ########## 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: OK ---------------------------------------------------------------- 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_r401355792 ########## 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: OK, 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_r401356003 ########## 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: No, 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_r401356033 ########## 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: 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_r401356054 ########## 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: 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_r401356180 ########## 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: OK ---------------------------------------------------------------- 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_r401356360 ########## 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: OK ---------------------------------------------------------------- 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_r401356901 ########## 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: OK ---------------------------------------------------------------- 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_r401357265 ########## 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: the 'isChildTableForMV' is a method of old mv implementation, it will be removed when clean old mv implementation, 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_r401357702 ########## 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: OK ---------------------------------------------------------------- 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_r401377301 ########## File path: core/src/main/java/org/apache/carbondata/core/view/MaterializedViewCatalog.java ########## @@ -0,0 +1,47 @@ +/* Review comment: we can start a new work to extract mv code to a single module after this 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_r401377408 ########## File path: mv/plan/src/main/scala/org/apache/carbondata/mv/plans/modular/Harmonizer.scala ########## @@ -72,8 +69,8 @@ object HarmonizeDimensionTable extends Rule[ModularPlan] with PredicateHelper { case s@Select(_, _, _, _, jedges, fact :: dims, _, _, _, _) if jedges.forall(e => e.joinType == LeftOuter || e.joinType == Inner) && fact.isInstanceOf[ModularRelation] && - dims.filterNot(_.isInstanceOf[modular.LeafNode]).nonEmpty && - dims.forall(d => (d.isInstanceOf[ModularRelation] || HarmonizedRelation.canHarmonize(d))) => { + !dims.forall(_.isInstanceOf[modular.LeafNode]) && Review comment: OK ---------------------------------------------------------------- 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_r401378493 ########## 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: OK ---------------------------------------------------------------- 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_r401380863 ########## 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: OK ---------------------------------------------------------------- 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_r401415307 ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/command/SIRebuildSegmentRunner.scala ########## @@ -35,32 +34,24 @@ import org.apache.carbondata.common.exceptions.sql.MalformedCarbonCommandExcepti import org.apache.carbondata.common.logging.LogServiceFactory import org.apache.carbondata.core.locks.{CarbonLockFactory, LockUsage} import org.apache.carbondata.core.metadata.{CarbonTableIdentifier, ColumnarFormatVersion} -import org.apache.carbondata.core.metadata.schema.table.{CarbonTable, TableInfo} +import org.apache.carbondata.core.metadata.schema.table.CarbonTable import org.apache.carbondata.core.statusmanager.{LoadMetadataDetails, SegmentStatus, SegmentStatusManager} import org.apache.carbondata.core.util.CarbonUtil import org.apache.carbondata.events.{OperationContext, OperationListenerBus} -case class SIRebuildSegmentCommand( - alterTableModel: AlterTableModel, - tableInfoOp: Option[TableInfo] = None, - operationContext: OperationContext = new OperationContext) - extends AtomicRunnableCommand { +case class SIRebuildSegmentRunner( Review comment: 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 |
Free forum by Nabble | Edit this page |