[GitHub] [carbondata] Karan980 commented on a change in pull request #4070: [CARBONDATA-4082] Fix alter table add segment query on adding a segment having delete delta files.
Karan980 commented on a change in pull request #4070: URL: https://github.com/apache/carbondata/pull/4070#discussion_r561607408 ## File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/CarbonAddLoadCommand.scala ## @@ -294,6 +297,49 @@ case class CarbonAddLoadCommand( OperationListenerBus.getInstance().fireEvent(loadTablePreStatusUpdateEvent, operationContext) } +val deltaFiles = FileFactory.getCarbonFile(segmentPath).listFiles() Review comment: Now, i'm taking just one delete delta file with highest timestamp for each block, by assuming that threshold for horizontal compaction is not changed from default. In case of SDK, all files are valid as horizontal compaction is not there in SDK 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] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
CarbonDataQA2 commented on pull request #4076: URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764355741 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] Indhumathi27 closed pull request #4079: [TEST]test ci
Indhumathi27 closed pull request #4079: URL: https://github.com/apache/carbondata/pull/4079 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] QiangCai commented on pull request #4078: [CARBONDATA-4075] Refactor code to use withEvents instead of fireEvent
QiangCai commented on pull request #4078: URL: https://github.com/apache/carbondata/pull/4078#issuecomment-765053051 retest this please 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 pull request #4064: [CARBONDATA-4096] SDK read fails from cluster and sdk read filter query on sort column giving wrong result with IndexServer
kunal642 commented on pull request #4064: URL: https://github.com/apache/carbondata/pull/4064#issuecomment-764530852 LGTM 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] Indhumathi27 commented on a change in pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Indhumathi27 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r561693339
##
File path:
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonDropMVCommand.scala
##
@@ -90,6 +95,37 @@ case class CarbonDropMVCommand(
}
}
+// Update the mvExists and related databases property to mv fact tables
+schema.getRelatedTables.asScala.foreach { table =>
+ val dbName = table.getDatabaseName
+ val tableName = table.getTableName
+ try {
+val carbonTable =
+ CarbonEnv.getCarbonTable(Some(dbName), tableName)(session)
+val relatedMVTablesMap = carbonTable.getMVTablesMap
+// check if database has materialized views or not
+val anotherMVExistsInDb =
viewManager.getSchemas(databaseName).asScala.exists {
+ mvSchema =>
+
mvSchema.getRelatedTables.asScala.exists(_.getTableName.equalsIgnoreCase(tableName))
+}
+if (!anotherMVExistsInDb) {
+ // If database dont have any MV, then remove the database from
related tables
+ // property and update table property of fact table
+ relatedMVTablesMap.get(databaseName).remove(name)
+ if (relatedMVTablesMap.get(databaseName).isEmpty) {
+relatedMVTablesMap.remove(databaseName)
+ }
+ CarbonIndexUtil.addOrModifyTableProperty(carbonTable,
+Map("relatedMVTablesMap".toLowerCase -> new
Gson().toJson(relatedMVTablesMap)),
+needLock = false)(session)
+ CarbonHiveIndexMetadataUtil.refreshTable(dbName, tableName,
session)
+}
+ } catch {
+case _: Exception =>
+// ignore as table is a non-carbon table
Review comment:
This Exception is added, because for parquet and orc, carbon table will
not be present.
##
File path: core/src/main/java/org/apache/carbondata/core/view/MVProvider.java
##
@@ -569,14 +568,31 @@ private synchronized void touchMDTFile() throws
IOException {
FileFactory.createDirectoryAndSetPermission(this.systemDirectory,
new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
}
- CarbonFile schemaIndexFile =
FileFactory.getCarbonFile(this.schemaIndexFilePath);
- if (schemaIndexFile.exists()) {
-schemaIndexFile.delete();
+ // two or more JVM process can access this method to update last
modified time at same
+ // time causing exception. So take a system level lock on system folder
and update
+ // last modified time of schema index file
+ ICarbonLock systemDirLock = CarbonLockFactory
+ .getSystemLevelCarbonLockObj(this.systemDirectory,
+ LockUsage.MATERIALIZED_VIEW_STATUS_LOCK);
+ boolean locked = false;
+ try {
+locked = systemDirLock.lockWithRetries();
+if (locked) {
+ CarbonFile schemaIndexFile =
FileFactory.getCarbonFile(this.schemaIndexFilePath);
+ if (schemaIndexFile.exists()) {
+schemaIndexFile.delete();
+ }
+ schemaIndexFile.createNewFile(new FsPermission(FsAction.ALL,
FsAction.ALL, FsAction.ALL));
+ this.lastModifiedTime = schemaIndexFile.getLastModifiedTime();
+} else {
+ LOG.warn("Unable to get Lock to refresh schema index last modified
time");
Review comment:
Not needed, As next operation will compare last modified time and update
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:
us...@infra.apache.org
[GitHub] [carbondata] akkio-97 commented on a change in pull request #4073: [CARBONDATA-4104] Vector filling for complex decimal type needs to be handled
akkio-97 commented on a change in pull request #4073:
URL: https://github.com/apache/carbondata/pull/4073#discussion_r561605603
##
File path:
core/src/main/java/org/apache/carbondata/core/metadata/datatype/DecimalConverterFactory.java
##
@@ -328,9 +328,29 @@ public BigDecimal getDecimal(Object valueToBeConverted) {
public void fillVector(Object valuesToBeConverted, int size,
ColumnVectorInfo vectorInfo, BitSet nullBitSet, DataType pageType) {
CarbonColumnVector vector = getCarbonColumnVector(vectorInfo,
nullBitSet);
- //TODO handle complex child
- int precision = vectorInfo.measure.getMeasure().getPrecision();
- int newMeasureScale = vectorInfo.measure.getMeasure().getScale();
+ int precision;
Review comment:
done
##
File path:
core/src/main/java/org/apache/carbondata/core/metadata/datatype/DecimalConverterFactory.java
##
@@ -348,6 +368,32 @@ public void fillVector(Object valuesToBeConverted, int
size,
vector.putDecimal(i, value, precision);
}
}
+ } else if (valuesToBeConverted instanceof byte[]) {
Review comment:
done
##
File path:
integration/spark/src/test/scala/org/apache/carbondata/integration/spark/testsuite/dataload/SparkStoreCreatorForPresto.scala
##
@@ -365,6 +365,15 @@ class SparkStoreCreatorForPresto extends QueryTest with
BeforeAndAfterAll{
sql(s"""LOAD DATA LOCAL INPATH '$resourcesPath/IUD/dest.csv' INTO TABLE
streaming_table""")
}
+ test("Test decimal unscaled converter") {
+sql("drop table if exists array_decimal")
+sql(
+ "CREATE TABLE IF NOT EXISTS array_decimal (salary array)
STORED AS " +
+ "carbondata"
+)
+sql("insert into array_decimal select array(922.580, 3.435) ")
Review comment:
done
##
File path:
integration/presto/src/test/scala/org/apache/carbondata/presto/integrationtest/PrestoTestUsingSparkStore.scala
##
@@ -325,4 +326,22 @@ class PrestoTestUsingSparkStore
}
+ test("Test decimal unscaled converter") {
+prestoServer
+ .execute(
+"create table presto_spark_db.array_decimal(salary
array(decimal(20,3)) ) with" +
+"(format='CARBON') ")
+copyStoreContents("array_decimal")
+val result: List[Map[String, Any]] = prestoServer
+ .executeQuery("SELECT * FROM presto_spark_db.array_decimal")
+assert(result.size == 1)
+for (i <- 0 to 0) {
Review comment:
done
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] asfgit closed pull request #4018: [CARBONDATA-4055]Fix creation of empty segment directory and meta entry when there is no update/insert data
asfgit closed pull request #4018: URL: https://github.com/apache/carbondata/pull/4018 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] Karan980 commented on pull request #4062: [CARBONDATA-4097] ColumnVectors should not be initialized as ColumnVectorWrapperDirect for alter tables.
Karan980 commented on pull request #4062: URL: https://github.com/apache/carbondata/pull/4062#issuecomment-764436367 retest this please 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] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
akashrn5 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r561671488
##
File path: core/src/main/java/org/apache/carbondata/core/view/MVProvider.java
##
@@ -569,14 +568,31 @@ private synchronized void touchMDTFile() throws
IOException {
FileFactory.createDirectoryAndSetPermission(this.systemDirectory,
new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
}
- CarbonFile schemaIndexFile =
FileFactory.getCarbonFile(this.schemaIndexFilePath);
- if (schemaIndexFile.exists()) {
-schemaIndexFile.delete();
+ // two or more JVM process can access this method to update last
modified time at same
+ // time causing exception. So take a system level lock on system folder
and update
+ // last modified time of schema index file
+ ICarbonLock systemDirLock = CarbonLockFactory
+ .getSystemLevelCarbonLockObj(this.systemDirectory,
+ LockUsage.MATERIALIZED_VIEW_STATUS_LOCK);
+ boolean locked = false;
+ try {
+locked = systemDirLock.lockWithRetries();
+if (locked) {
+ CarbonFile schemaIndexFile =
FileFactory.getCarbonFile(this.schemaIndexFilePath);
+ if (schemaIndexFile.exists()) {
+schemaIndexFile.delete();
+ }
+ schemaIndexFile.createNewFile(new FsPermission(FsAction.ALL,
FsAction.ALL, FsAction.ALL));
+ this.lastModifiedTime = schemaIndexFile.getLastModifiedTime();
+} else {
+ LOG.warn("Unable to get Lock to refresh schema index last modified
time");
Review comment:
if lock acquire fails, just a warn, dont we need to fail? as it may lead
to some other problem?
##
File path: core/src/main/java/org/apache/carbondata/core/view/MVManager.java
##
@@ -132,6 +108,31 @@ public boolean isMVInSyncWithParentTables(MVSchema
mvSchema) throws IOException
return schemas;
}
+ /**
+ * It gives all mv schemas from given databases in the store
+ */
+ public List getSchemas(Map> mvTablesMap)
throws IOException {
+List schemas = new ArrayList<>();
+for (Map.Entry> databaseEntry :
mvTablesMap.entrySet()) {
+ String database = databaseEntry.getKey();
+ List mvTables = databaseEntry.getValue();
+ for (String mvTable : mvTables) {
+try {
+ schemas.add(this.getSchema(database, mvTable));
+} catch (IOException ex) {
+ throw ex;
Review comment:
please add a error log here, before throwing exception
##
File path:
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonDropMVCommand.scala
##
@@ -90,6 +95,37 @@ case class CarbonDropMVCommand(
}
}
+// Update the mvExists and related databases property to mv fact tables
+schema.getRelatedTables.asScala.foreach { table =>
+ val dbName = table.getDatabaseName
+ val tableName = table.getTableName
+ try {
+val carbonTable =
+ CarbonEnv.getCarbonTable(Some(dbName), tableName)(session)
+val relatedMVTablesMap = carbonTable.getMVTablesMap
+// check if database has materialized views or not
+val anotherMVExistsInDb =
viewManager.getSchemas(databaseName).asScala.exists {
+ mvSchema =>
+
mvSchema.getRelatedTables.asScala.exists(_.getTableName.equalsIgnoreCase(tableName))
+}
+if (!anotherMVExistsInDb) {
+ // If database dont have any MV, then remove the database from
related tables
+ // property and update table property of fact table
+ relatedMVTablesMap.get(databaseName).remove(name)
+ if (relatedMVTablesMap.get(databaseName).isEmpty) {
+relatedMVTablesMap.remove(databaseName)
+ }
+ CarbonIndexUtil.addOrModifyTableProperty(carbonTable,
+Map("relatedMVTablesMap".toLowerCase -> new
Gson().toJson(relatedMVTablesMap)),
+needLock = false)(session)
+ CarbonHiveIndexMetadataUtil.refreshTable(dbName, tableName,
session)
+}
+ } catch {
+case _: Exception =>
+// ignore as table is a non-carbon table
Review comment:
this comment is wrong? this flow will be for carbon tables also right?
##
File path:
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonCreateMVCommand.scala
##
@@ -122,6 +130,30 @@ case class CarbonCreateMVCommand(
this.viewSchema = schema
+// Update the mvExists and related databases property to mv fact tables
+relatedTableList.asScala.foreach { parentTable =>
+ val dbName = parentTable.getDatabaseName
+ val tableName = parentTable.getTableName
+ try {
+val carbonTable = CarbonEnv.getCarbonTable(Some(dbName),
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4078: [CARBONDATA-4075] Refactor code to use withEvents instead of fireEvent
CarbonDataQA2 commented on pull request #4078: URL: https://github.com/apache/carbondata/pull/4078#issuecomment-765088260 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] Indhumathi27 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Indhumathi27 commented on pull request #4076: URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764288658 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 pull request #4018: [CARBONDATA-4055]Fix creation of empty segment directory and meta entry when there is no update/insert data
kunal642 commented on pull request #4018: URL: https://github.com/apache/carbondata/pull/4018#issuecomment-764338208 LGTM 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 pull request #4062: [CARBONDATA-4097] ColumnVectors should not be initialized as ColumnVectorWrapperDirect for alter tables.
kunal642 commented on pull request #4062: URL: https://github.com/apache/carbondata/pull/4062#issuecomment-764380634 retest this please 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] CarbonDataQA2 commented on pull request #4079: [TEST]test ci
CarbonDataQA2 commented on pull request #4079: URL: https://github.com/apache/carbondata/pull/4079#issuecomment-764480479 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] CarbonDataQA2 commented on pull request #4062: [CARBONDATA-4097] ColumnVectors should not be initialized as ColumnVectorWrapperDirect for alter tables.
CarbonDataQA2 commented on pull request #4062: URL: https://github.com/apache/carbondata/pull/4062#issuecomment-764431030 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] CarbonDataQA2 commented on pull request #4078: [CARBONDATA-4075] Refactor code to use withEvents instead of fireEvent
CarbonDataQA2 commented on pull request #4078: URL: https://github.com/apache/carbondata/pull/4078#issuecomment-765091491 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3573/ 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] CarbonDataQA2 commented on pull request #4078: [CARBONDATA-4075] Refactor code to use withEvents instead of fireEvent
CarbonDataQA2 commented on pull request #4078: URL: https://github.com/apache/carbondata/pull/4078#issuecomment-765088260 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5333/ 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] QiangCai commented on pull request #4078: [CARBONDATA-4075] Refactor code to use withEvents instead of fireEvent
QiangCai commented on pull request #4078: URL: https://github.com/apache/carbondata/pull/4078#issuecomment-765053051 retest this please 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] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
CarbonDataQA2 commented on pull request #4076: URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764825371 Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3572/ 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] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
CarbonDataQA2 commented on pull request #4076: URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764822381 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5332/ 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] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
CarbonDataQA2 commented on pull request #4076: URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764626476 Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3571/ 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] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
CarbonDataQA2 commented on pull request #4076: URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764623559 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5331/ 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] CarbonDataQA2 commented on pull request #4079: [TEST]test ci
CarbonDataQA2 commented on pull request #4079: URL: https://github.com/apache/carbondata/pull/4079#issuecomment-764537279 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3570/ 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] Indhumathi27 closed pull request #4079: [TEST]test ci
Indhumathi27 closed pull request #4079: URL: https://github.com/apache/carbondata/pull/4079 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] CarbonDataQA2 commented on pull request #4079: [TEST]test ci
CarbonDataQA2 commented on pull request #4079: URL: https://github.com/apache/carbondata/pull/4079#issuecomment-764534625 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5330/ 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 pull request #4064: [CARBONDATA-4096] SDK read fails from cluster and sdk read filter query on sort column giving wrong result with IndexServer
kunal642 commented on pull request #4064: URL: https://github.com/apache/carbondata/pull/4064#issuecomment-764530852 LGTM 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] Indhumathi27 commented on a change in pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Indhumathi27 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r561714909
##
File path: core/src/main/java/org/apache/carbondata/core/view/MVProvider.java
##
@@ -569,14 +568,31 @@ private synchronized void touchMDTFile() throws
IOException {
FileFactory.createDirectoryAndSetPermission(this.systemDirectory,
new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
}
- CarbonFile schemaIndexFile =
FileFactory.getCarbonFile(this.schemaIndexFilePath);
- if (schemaIndexFile.exists()) {
-schemaIndexFile.delete();
+ // two or more JVM process can access this method to update last
modified time at same
+ // time causing exception. So take a system level lock on system folder
and update
+ // last modified time of schema index file
+ ICarbonLock systemDirLock = CarbonLockFactory
+ .getSystemLevelCarbonLockObj(this.systemDirectory,
+ LockUsage.MATERIALIZED_VIEW_STATUS_LOCK);
+ boolean locked = false;
+ try {
+locked = systemDirLock.lockWithRetries();
+if (locked) {
+ CarbonFile schemaIndexFile =
FileFactory.getCarbonFile(this.schemaIndexFilePath);
+ if (schemaIndexFile.exists()) {
+schemaIndexFile.delete();
+ }
+ schemaIndexFile.createNewFile(new FsPermission(FsAction.ALL,
FsAction.ALL, FsAction.ALL));
+ this.lastModifiedTime = schemaIndexFile.getLastModifiedTime();
+} else {
+ LOG.warn("Unable to get Lock to refresh schema index last modified
time");
Review comment:
Not needed, As next operation will compare last modified time and update
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:
us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4062: [CARBONDATA-4097] ColumnVectors should not be initialized as ColumnVectorWrapperDirect for alter tables.
CarbonDataQA2 commented on pull request #4062: URL: https://github.com/apache/carbondata/pull/4062#issuecomment-764486033 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3567/ 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] CarbonDataQA2 commented on pull request #4079: [TEST]test ci
CarbonDataQA2 commented on pull request #4079: URL: https://github.com/apache/carbondata/pull/4079#issuecomment-764481057 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5329/ 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] CarbonDataQA2 commented on pull request #4079: [TEST]test ci
CarbonDataQA2 commented on pull request #4079: URL: https://github.com/apache/carbondata/pull/4079#issuecomment-764480479 Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3569/ 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] CarbonDataQA2 commented on pull request #4062: [CARBONDATA-4097] ColumnVectors should not be initialized as ColumnVectorWrapperDirect for alter tables.
CarbonDataQA2 commented on pull request #4062: URL: https://github.com/apache/carbondata/pull/4062#issuecomment-764480017 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5327/ 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] Indhumathi27 opened a new pull request #4079: [TEST]test ci
Indhumathi27 opened a new pull request #4079: URL: https://github.com/apache/carbondata/pull/4079 ### Why is this PR needed? ### What changes were proposed in this PR? ### Does this PR introduce any user interface change? - No - Yes. (please explain the change and update document) ### Is any new testcase added? - No - Yes 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] Indhumathi27 commented on a change in pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Indhumathi27 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r561693339
##
File path:
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonDropMVCommand.scala
##
@@ -90,6 +95,37 @@ case class CarbonDropMVCommand(
}
}
+// Update the mvExists and related databases property to mv fact tables
+schema.getRelatedTables.asScala.foreach { table =>
+ val dbName = table.getDatabaseName
+ val tableName = table.getTableName
+ try {
+val carbonTable =
+ CarbonEnv.getCarbonTable(Some(dbName), tableName)(session)
+val relatedMVTablesMap = carbonTable.getMVTablesMap
+// check if database has materialized views or not
+val anotherMVExistsInDb =
viewManager.getSchemas(databaseName).asScala.exists {
+ mvSchema =>
+
mvSchema.getRelatedTables.asScala.exists(_.getTableName.equalsIgnoreCase(tableName))
+}
+if (!anotherMVExistsInDb) {
+ // If database dont have any MV, then remove the database from
related tables
+ // property and update table property of fact table
+ relatedMVTablesMap.get(databaseName).remove(name)
+ if (relatedMVTablesMap.get(databaseName).isEmpty) {
+relatedMVTablesMap.remove(databaseName)
+ }
+ CarbonIndexUtil.addOrModifyTableProperty(carbonTable,
+Map("relatedMVTablesMap".toLowerCase -> new
Gson().toJson(relatedMVTablesMap)),
+needLock = false)(session)
+ CarbonHiveIndexMetadataUtil.refreshTable(dbName, tableName,
session)
+}
+ } catch {
+case _: Exception =>
+// ignore as table is a non-carbon table
Review comment:
This Exception is added, because for parquet and orc, carbon table will
not be present.
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] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
CarbonDataQA2 commented on pull request #4076: URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764465842 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5328/ 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] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
CarbonDataQA2 commented on pull request #4076: URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764464803 Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3568/ 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] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
akashrn5 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r561671488
##
File path: core/src/main/java/org/apache/carbondata/core/view/MVProvider.java
##
@@ -569,14 +568,31 @@ private synchronized void touchMDTFile() throws
IOException {
FileFactory.createDirectoryAndSetPermission(this.systemDirectory,
new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
}
- CarbonFile schemaIndexFile =
FileFactory.getCarbonFile(this.schemaIndexFilePath);
- if (schemaIndexFile.exists()) {
-schemaIndexFile.delete();
+ // two or more JVM process can access this method to update last
modified time at same
+ // time causing exception. So take a system level lock on system folder
and update
+ // last modified time of schema index file
+ ICarbonLock systemDirLock = CarbonLockFactory
+ .getSystemLevelCarbonLockObj(this.systemDirectory,
+ LockUsage.MATERIALIZED_VIEW_STATUS_LOCK);
+ boolean locked = false;
+ try {
+locked = systemDirLock.lockWithRetries();
+if (locked) {
+ CarbonFile schemaIndexFile =
FileFactory.getCarbonFile(this.schemaIndexFilePath);
+ if (schemaIndexFile.exists()) {
+schemaIndexFile.delete();
+ }
+ schemaIndexFile.createNewFile(new FsPermission(FsAction.ALL,
FsAction.ALL, FsAction.ALL));
+ this.lastModifiedTime = schemaIndexFile.getLastModifiedTime();
+} else {
+ LOG.warn("Unable to get Lock to refresh schema index last modified
time");
Review comment:
if lock acquire fails, just a warn, dont we need to fail? as it may lead
to some other problem?
##
File path: core/src/main/java/org/apache/carbondata/core/view/MVManager.java
##
@@ -132,6 +108,31 @@ public boolean isMVInSyncWithParentTables(MVSchema
mvSchema) throws IOException
return schemas;
}
+ /**
+ * It gives all mv schemas from given databases in the store
+ */
+ public List getSchemas(Map> mvTablesMap)
throws IOException {
+List schemas = new ArrayList<>();
+for (Map.Entry> databaseEntry :
mvTablesMap.entrySet()) {
+ String database = databaseEntry.getKey();
+ List mvTables = databaseEntry.getValue();
+ for (String mvTable : mvTables) {
+try {
+ schemas.add(this.getSchema(database, mvTable));
+} catch (IOException ex) {
+ throw ex;
Review comment:
please add a error log here, before throwing exception
##
File path:
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonDropMVCommand.scala
##
@@ -90,6 +95,37 @@ case class CarbonDropMVCommand(
}
}
+// Update the mvExists and related databases property to mv fact tables
+schema.getRelatedTables.asScala.foreach { table =>
+ val dbName = table.getDatabaseName
+ val tableName = table.getTableName
+ try {
+val carbonTable =
+ CarbonEnv.getCarbonTable(Some(dbName), tableName)(session)
+val relatedMVTablesMap = carbonTable.getMVTablesMap
+// check if database has materialized views or not
+val anotherMVExistsInDb =
viewManager.getSchemas(databaseName).asScala.exists {
+ mvSchema =>
+
mvSchema.getRelatedTables.asScala.exists(_.getTableName.equalsIgnoreCase(tableName))
+}
+if (!anotherMVExistsInDb) {
+ // If database dont have any MV, then remove the database from
related tables
+ // property and update table property of fact table
+ relatedMVTablesMap.get(databaseName).remove(name)
+ if (relatedMVTablesMap.get(databaseName).isEmpty) {
+relatedMVTablesMap.remove(databaseName)
+ }
+ CarbonIndexUtil.addOrModifyTableProperty(carbonTable,
+Map("relatedMVTablesMap".toLowerCase -> new
Gson().toJson(relatedMVTablesMap)),
+needLock = false)(session)
+ CarbonHiveIndexMetadataUtil.refreshTable(dbName, tableName,
session)
+}
+ } catch {
+case _: Exception =>
+// ignore as table is a non-carbon table
Review comment:
this comment is wrong? this flow will be for carbon tables also right?
##
File path:
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonCreateMVCommand.scala
##
@@ -122,6 +130,30 @@ case class CarbonCreateMVCommand(
this.viewSchema = schema
+// Update the mvExists and related databases property to mv fact tables
+relatedTableList.asScala.foreach { parentTable =>
+ val dbName = parentTable.getDatabaseName
+ val tableName = parentTable.getTableName
+ try {
+val carbonTable = CarbonEnv.getCarbonTable(Some(dbName),
