subject:"\[GitHub\] \[carbondata\] Indhumathi27 commented on a change in pull request #3977\: \[CARBONDATA\-4027\] Fix the wrong modifiedtime of loading files in inse…"

[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #3977: [CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse…

2020-10-23 Thread GitBox



Indhumathi27 commented on a change in pull request #3977:
URL: https://github.com/apache/carbondata/pull/3977#discussion_r510762119



##
File path: 
integration/spark/src/main/scala/org/apache/spark/sql/events/MergeIndexEventListener.scala
##
@@ -43,11 +43,6 @@ class MergeIndexEventListener extends OperationEventListener 
with Logging {
   override def onEvent(event: Event, operationContext: OperationContext): Unit 
= {
 event match {
   case preStatusUpdateEvent: LoadTablePreStatusUpdateEvent =>
-// skip merge index in case of insert stage flow
-if (null != 
operationContext.getProperty(CarbonCommonConstants.IS_INSERT_STAGE) &&

Review comment:
   This property also has to be removed from CarbonCommonConstants, as no 
more usage will be found





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 #3977: [CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse…

2020-10-13 Thread GitBox



Indhumathi27 commented on a change in pull request #3977:
URL: https://github.com/apache/carbondata/pull/3977#discussion_r504403662



##
File path: 
integration/spark/src/main/scala/org/apache/spark/sql/hive/CarbonFileMetastore.scala
##
@@ -430,9 +430,8 @@ class CarbonFileMetastore extends CarbonMetaStore {
 thriftWriter.open(FileWriteOperation.OVERWRITE)
 thriftWriter.write(thriftTableInfo)
 thriftWriter.close()
-val modifiedTime = System.currentTimeMillis()
-FileFactory.getCarbonFile(schemaFilePath).setLastModifiedTime(modifiedTime)
-updateSchemasUpdatedTime(identifier.getCarbonTableIdentifier.getTableId, 
modifiedTime)
+updateSchemasUpdatedTime(identifier.getCarbonTableIdentifier.getTableId,
+  System.currentTimeMillis())

Review comment:
   I think, modified time should be the lastModifiedTime of the schema file





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 #3977: [CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse…

2020-10-13 Thread GitBox



Indhumathi27 commented on a change in pull request #3977:
URL: https://github.com/apache/carbondata/pull/3977#discussion_r503982610



##
File path: core/src/main/java/org/apache/carbondata/core/view/MVProvider.java
##
@@ -545,14 +545,14 @@ private void touchMDTFile() throws IOException {
 FileFactory.createDirectoryAndSetPermission(this.systemDirectory,
 new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
   }
-  if (!FileFactory.isFileExist(this.schemaIndexFilePath)) {
-FileFactory.createNewFile(
-this.schemaIndexFilePath,
-new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
+  if (FileFactory.isFileExist(this.schemaIndexFilePath)) {

Review comment:
   ```suggestion
 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();
   ```





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 #3977: [CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse…

2020-10-13 Thread GitBox



Indhumathi27 commented on a change in pull request #3977:
URL: https://github.com/apache/carbondata/pull/3977#discussion_r503947136



##
File path: 
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/CarbonInsertFromStageCommand.scala
##
@@ -533,14 +533,13 @@ case class CarbonInsertFromStageCommand(
 val stageLoadingFile =
   FileFactory.getCarbonFile(stagePath +
 File.separator + files._1.getName + 
CarbonTablePath.LOADING_FILE_SUFFIX);
-// Try to create loading files
-// make isFailed to be true if createNewFile return false.
-// the reason can be file exists or exceptions.
-var isFailed = !stageLoadingFile.createNewFile()
-// if file exists, modify the lastmodifiedtime of the file.
-if (isFailed) {
-  // make isFailed to be true if setLastModifiedTime return false.
-  isFailed = 
!stageLoadingFile.setLastModifiedTime(System.currentTimeMillis());
+// Try to recreate loading files if the loading file exists
+// or create loading files directly if the loading file doesn't 
exist
+// set isFailed to be false when (delete and) createfile success
+var isFailed = if (stageLoadingFile.exists()) {

Review comment:
   ```suggestion
   val isFailed = if (stageLoadingFile.exists()) {
   ```





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 #3977: [CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse…

2020-10-12 Thread GitBox



Indhumathi27 commented on a change in pull request #3977:
URL: https://github.com/apache/carbondata/pull/3977#discussion_r503670575



##
File path: 
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/CarbonInsertFromStageCommand.scala
##
@@ -533,14 +533,13 @@ case class CarbonInsertFromStageCommand(
 val stageLoadingFile =
   FileFactory.getCarbonFile(stagePath +
 File.separator + files._1.getName + 
CarbonTablePath.LOADING_FILE_SUFFIX);
-// Try to create loading files
-// make isFailed to be true if createNewFile return false.
-// the reason can be file exists or exceptions.
-var isFailed = !stageLoadingFile.createNewFile()
-// if file exists, modify the lastmodifiedtime of the file.
-if (isFailed) {
-  // make isFailed to be true if setLastModifiedTime return false.
-  isFailed = 
!stageLoadingFile.setLastModifiedTime(System.currentTimeMillis());
+// Try to recreate loading files if the loading file exists
+// or create loading files directly if the loading file doesn't 
exist
+// set isFailed to be false when (delete and) createfile success
+var isFailed = if (stageLoadingFile.exists()) {

Review comment:
   isFailed will always be false. can remove it and update the comment





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 #3977: [CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse…

2020-10-12 Thread GitBox



Indhumathi27 commented on a change in pull request #3977:
URL: https://github.com/apache/carbondata/pull/3977#discussion_r503670575



##
File path: 
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/CarbonInsertFromStageCommand.scala
##
@@ -533,14 +533,13 @@ case class CarbonInsertFromStageCommand(
 val stageLoadingFile =
   FileFactory.getCarbonFile(stagePath +
 File.separator + files._1.getName + 
CarbonTablePath.LOADING_FILE_SUFFIX);
-// Try to create loading files
-// make isFailed to be true if createNewFile return false.
-// the reason can be file exists or exceptions.
-var isFailed = !stageLoadingFile.createNewFile()
-// if file exists, modify the lastmodifiedtime of the file.
-if (isFailed) {
-  // make isFailed to be true if setLastModifiedTime return false.
-  isFailed = 
!stageLoadingFile.setLastModifiedTime(System.currentTimeMillis());
+// Try to recreate loading files if the loading file exists
+// or create loading files directly if the loading file doesn't 
exist
+// set isFailed to be false when (delete and) createfile success
+var isFailed = if (stageLoadingFile.exists()) {

Review comment:
   isFailed will always be false. can 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:
us...@infra.apache.org

[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #3977: [CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse…

[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #3977: [CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse…

[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #3977: [CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse…

[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #3977: [CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse…

[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #3977: [CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse…

[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #3977: [CARBONDATA-4027] Fix the wrong modifiedtime of loading files in inse…

6 matches

Site Navigation

Mail list logo

Footer information