niuge01 commented on a change in pull request #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#discussion_r396975590 ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/listeners/MVListeners.scala ########## @@ -67,7 +67,7 @@ object AlterDataMaptableCompactionPostListener extends OperationEventListener { val allDataMapSchemas = DataMapStoreManager.getInstance .getDataMapSchemasOfTable(carbonTable).asScala .filter(dataMapSchema => null != dataMapSchema.getRelationIdentifier && - !dataMapSchema.isIndexDataMap) + !dataMapSchema.isIndex) Review comment: Yes, will be delete when clean the old mv implementation ---------------------------------------------------------------- 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: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#discussion_r396975897 ########## 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: MVParser will be delete when clean the old mv implementation ---------------------------------------------------------------- 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: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#discussion_r396975897 ########## 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: MVParser never used now, MVParser will be delete when clean the old mv implementation ---------------------------------------------------------------- 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: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#discussion_r396976400 ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/index/CarbonCreateIndexCommand.scala ########## @@ -0,0 +1,195 @@ +/* + * 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.spark.sql.execution.command.index + +import scala.collection.JavaConverters._ + +import org.apache.spark.sql._ +import org.apache.spark.sql.catalyst.TableIdentifier +import org.apache.spark.sql.execution.command._ +import org.apache.spark.sql.secondaryindex.command.IndexModel + +import org.apache.carbondata.common.exceptions.sql.{MalformedCarbonCommandException, MalformedIndexCommandException, NoSuchIndexException} +import org.apache.carbondata.common.logging.LogServiceFactory +import org.apache.carbondata.core.constants.CarbonCommonConstants +import org.apache.carbondata.core.datamap.DataMapStoreManager +import org.apache.carbondata.core.datamap.status.DataMapStatusManager +import org.apache.carbondata.core.metadata.ColumnarFormatVersion +import org.apache.carbondata.core.metadata.datatype.DataTypes +import org.apache.carbondata.core.metadata.schema.datamap.{DataMapClassProvider, DataMapProperty} +import org.apache.carbondata.core.metadata.schema.table.{CarbonTable, DataMapSchema} +import org.apache.carbondata.core.util.{CarbonProperties, CarbonUtil} +import org.apache.carbondata.datamap.IndexProvider +import org.apache.carbondata.events._ + +/** + * Below command class will be used to create index on table + * and updating the parent table about the index information + */ +case class CarbonCreateIndexCommand( + indexModel: IndexModel, + indexProviderName: String, + properties: Map[String, String], + ifNotExistsSet: Boolean = false, + var deferredRebuild: Boolean = false) + extends AtomicRunnableCommand { + + private val LOGGER = LogServiceFactory.getLogService(this.getClass.getName) + private var provider: IndexProvider = _ + private var parentTable: CarbonTable = _ + private var dataMapSchema: DataMapSchema = _ + + override def processMetadata(sparkSession: SparkSession): Seq[Row] = { + // since streaming segment does not support building index yet, + // so streaming table does not support create index + parentTable = CarbonEnv.getCarbonTable(indexModel.dbName, indexModel.tableName)(sparkSession) + val indexName = indexModel.indexName + + setAuditTable(parentTable) + setAuditInfo(Map("provider" -> indexProviderName, "indexName" -> indexName) ++ properties) + + if (!parentTable.getTableInfo.isTransactionalTable) { + throw new MalformedCarbonCommandException("Unsupported operation on non transactional table") + } + + if (DataMapStoreManager.getInstance().isDataMapExist(parentTable.getTableId, indexName)) { + if (!ifNotExistsSet) { + throw new NoSuchIndexException(indexName) 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
CarbonDataQA1 commented on issue #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#issuecomment-603103936 Build Failed with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/838/ ---------------------------------------------------------------- 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
CarbonDataQA1 commented on issue #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#issuecomment-603136100 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2545/ ---------------------------------------------------------------- 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
CarbonDataQA1 commented on issue #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#issuecomment-603732720 Build Failed with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/847/ ---------------------------------------------------------------- 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
CarbonDataQA1 commented on issue #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#issuecomment-603733216 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2554/ ---------------------------------------------------------------- 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
CarbonDataQA1 commented on issue #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#issuecomment-603810828 Build Failed with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/848/ ---------------------------------------------------------------- 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
CarbonDataQA1 commented on issue #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#issuecomment-603813251 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2555/ ---------------------------------------------------------------- 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
jackylk commented on a change in pull request #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#discussion_r398280930 ########## File path: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/DiskBasedDMSchemaStorageProvider.java ########## @@ -172,17 +172,17 @@ public void dropSchema(String dataMapName) throws IOException { String schemaPath = getSchemaPath(storePath, dataMapName); if (!FileFactory.isFileExist(schemaPath)) { - throw new IOException("DataMap with name " + dataMapName + " does not exists in storage"); + throw new IOException("Index with name " + dataMapName + " does not exists in storage"); } - LOG.info(String.format("Trying to delete DataMap %s schema", dataMapName)); + LOG.info(String.format("Trying to delete Index %s schema", dataMapName)); Review comment: ```suggestion LOG.info(String.format("Trying to delete Index %s schema", indexName)); ``` ---------------------------------------------------------------- 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
CarbonDataQA1 commented on issue #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#issuecomment-604229889 Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/852/ ---------------------------------------------------------------- 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
CarbonDataQA1 commented on issue #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#issuecomment-604235438 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2559/ ---------------------------------------------------------------- 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: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#discussion_r399883644 ########## File path: core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java ########## @@ -1595,6 +1595,24 @@ public String getSystemFolderLocation() { return systemLocation + CarbonCommonConstants.FILE_SEPARATOR + "_system"; } + /** + * Get the configured system folder location. + * @return + */ + public String getSystemFolderLocation(String databaseName) { + String systemLocation = CarbonProperties.getInstance() 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
CarbonDataQA1 commented on issue #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#issuecomment-605770150 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/883/ ---------------------------------------------------------------- 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
CarbonDataQA1 commented on issue #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#issuecomment-605770848 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2591/ ---------------------------------------------------------------- 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
ajantha-bhat commented on a change in pull request #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#discussion_r399961269 ########## File path: common/src/main/java/org/apache/carbondata/common/exceptions/sql/MalformedIndexCommandException.java ########## @@ -21,21 +21,21 @@ import org.apache.carbondata.common.annotations.InterfaceStability; /** - * This exception will be thrown when Datamap related SQL statement is invalid + * This exception will be thrown when index related SQL statement is invalid Review comment: @niuge01 and @jackylk : As per my understanding. After this PR. There won't be datamap word in the code. It is either index or materialized view. But **many package names still has datamap** [exmaple CgDatamap, FgDatamap] and files also exist with datamap name. [Example DataMapStatus, DataMapStatusManager] Do we need to completly remove if weare doing this change. ---------------------------------------------------------------- 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
ajantha-bhat commented on a change in pull request #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#discussion_r393458926 ########## 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: better to rename the file itself ---------------------------------------------------------------- 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
ajantha-bhat commented on a change in pull request #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#discussion_r399961269 ########## File path: common/src/main/java/org/apache/carbondata/common/exceptions/sql/MalformedIndexCommandException.java ########## @@ -21,21 +21,21 @@ import org.apache.carbondata.common.annotations.InterfaceStability; /** - * This exception will be thrown when Datamap related SQL statement is invalid + * This exception will be thrown when index related SQL statement is invalid Review comment: @niuge01 and @jackylk : As per my understanding. After this PR. There won't be datamap word in the code. It is either index or materialized view. But **many package names still has datamap** [exmaple CgDatamap, FgDatamap] and **files also exist with datamap name**. [Example DataMapStatus, DataMapStatusManager] Do we need to completly remove if weare doing this change. ---------------------------------------------------------------- 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
ajantha-bhat commented on a change in pull request #3661: [WIP] Support materialized view
URL: https://github.com/apache/carbondata/pull/3661#discussion_r399962377 ########## File path: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ########## @@ -2265,6 +2265,11 @@ private CarbonCommonConstants() { */ public static final String PARENT_TABLES = "parent_tables"; + /** + * This property will be used to store table name's associated with mv + */ Review comment: why not clean in this PR itself as it is meant for MV refactoring ? ---------------------------------------------------------------- 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 |