[GitHub] carbondata pull request #1304: [CARBONDATA-1411] [CARBONDATA-1414] Show Segm...

[qiuchenjian-2]

GitHub user chenliang613 opened a pull request:

    https://github.com/apache/carbondata/pull/1304

    [CARBONDATA-1411] [CARBONDATA-1414] Show Segment command gives Null Pointer Exception after updation

    Fix issues while showing Segment command gives Null Pointer Exception after updation


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/chenliang613/carbondata jira1411

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/carbondata/pull/1304.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1304
   
----
commit 8575bfcd8337762bc0810c36f83e266928fc11cf
Author: chenliang613 <[hidden email]>
Date:   2017-08-31T03:49:11Z

    [CARBONDATA-1411] [CARBONDATA-1414] Show Segment command gives Null Pointer Exception after updation

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1304#discussion_r136243363
 
    --- Diff: integration/spark-common/src/main/java/org/apache/carbondata/spark/load/CarbonLoaderUtil.java ---
    @@ -258,92 +258,105 @@ public static void deleteLocalDataLoadFolderLocation(CarbonLoadModel loadModel,
        * @throws IOException
        */
       public static boolean recordLoadMetadata(LoadMetadataDetails newMetaEntry,
    -      CarbonLoadModel loadModel, boolean loadStartEntry, boolean insertOverwrite)
    +      CarbonLoadModel loadModel, boolean loadStartEntry, boolean insertOverwrite, boolean isUpdate)
    --- End diff --
   
    I think we may not require to change this code, In update case this method shouldn't be called


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1304#discussion_r136243428
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/spark/rdd/CarbonDataRDDFactory.scala ---
    @@ -1021,7 +1021,7 @@ object CarbonDataRDDFactory {
                 carbonLoadModel.getFactTimeStamp,
                 true)
             val success = CarbonLoaderUtil.recordLoadMetadata(metadataDetails,
    -          carbonLoadModel, false, overwriteTable)
    +          carbonLoadModel, false, overwriteTable, updateModel.isDefined)
    --- End diff --
   
    not require to change as update flow does not come here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1304#discussion_r136243668
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchema.scala ---
    @@ -774,8 +774,8 @@ case class LoadTable(
             GlobalDictionaryUtil.updateTableMetadataFunc = LoadTable.updateTableMetadata
             val storePath = relation.tableMeta.storePath
             // add the start entry for the new load in the table status file
    -        CommonUtil.
    -          readAndUpdateLoadProgressInTableMeta(carbonLoadModel, storePath, isOverwriteTable)
    +        CommonUtil.readAndUpdateLoadProgressInTableMeta(carbonLoadModel, storePath,
    --- End diff --
   
    Here we can add if check like below
    ```
    if (!updateModel.isDefined) {
    CommonUtil.readAndUpdateLoadProgressInTableMeta(carbonLoadModel, storePath, isOverwriteTable)
    }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user zzcclp commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1304#discussion_r136251853
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchema.scala ---
    @@ -774,8 +774,8 @@ case class LoadTable(
             GlobalDictionaryUtil.updateTableMetadataFunc = LoadTable.updateTableMetadata
             val storePath = relation.tableMeta.storePath
             // add the start entry for the new load in the table status file
    -        CommonUtil.
    -          readAndUpdateLoadProgressInTableMeta(carbonLoadModel, storePath, isOverwriteTable)
    +        CommonUtil.readAndUpdateLoadProgressInTableMeta(carbonLoadModel, storePath,
    --- End diff --
   
    Agree. But it need to read 'LoadMetadataDetails' when updateModel.isDefined == true, otherwise it will throw NPE because carbonLoadModel.getLoadMetadataDetails is null.
    Add code like below:
   
    ```
            if (!updateModel.isDefined) {
              CommonUtil.
                readAndUpdateLoadProgressInTableMeta(carbonLoadModel, storePath, isOverwriteTable)
            } else {
              CommonUtil.readLoadMetadataDetails(carbonLoadModel, storePath)
            }
    ```
    @ravipesala , it don't need to add a load meta recorder when execute update, right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user chenliang613 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1304#discussion_r136252580
 
    --- Diff: integration/spark-common/src/main/java/org/apache/carbondata/spark/load/CarbonLoaderUtil.java ---
    @@ -258,92 +258,105 @@ public static void deleteLocalDataLoadFolderLocation(CarbonLoadModel loadModel,
        * @throws IOException
        */
       public static boolean recordLoadMetadata(LoadMetadataDetails newMetaEntry,
    -      CarbonLoadModel loadModel, boolean loadStartEntry, boolean insertOverwrite)
    +      CarbonLoadModel loadModel, boolean loadStartEntry, boolean insertOverwrite, boolean isUpdate)
    --- End diff --
   
    Because should set carbonLoadModel in recordLoadMetadata(), otherwise the below code in carbonTableSchema.scala will has NLP issue:
    if (carbonLoadModel.getLoadMetadataDetails.isEmpty && carbonLoadModel.getUseOnePass &&
                  StringUtils.isEmpty(column_dict) && StringUtils.isEmpty(all_dictionary_path)) {
                LOGGER.info(s"Cannot use single_pass=true for $dbName.$tableName during the first load")
                LOGGER.audit(s"Cannot use single_pass=true for $dbName.$tableName during the first load")
                carbonLoadModel.setUseOnePass(false)
              }
   
    in carbonTableSchema.scala , the below code need to
   



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1304
 
    SDV Build Success with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/343/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/1304
 
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/1304
 
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/1304
 
    retest this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1304
 
    SDV Build Success with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/358/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/1304
 
    retest this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/1304
 
    test again


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/1304
 
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1304
 
    retest this please


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1304
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/548/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1304
 
    LGTM


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

    https://github.com/apache/carbondata/pull/1304


---