[GitHub] carbondata pull request #927: [CARBONDATA-1066] Fixed NO_INVERTED_INDEX for ...

[qiuchenjian-2]

GitHub user rahulforallp opened a pull request:

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

    [CARBONDATA-1066] Fixed NO_INVERTED_INDEX for other table properties and NPE in compaction

   
   
    1. Measure shouldn't supported inside no_inverted_index , if it is not included in sort_column or dictionary_include.
    2. Dimension excluded from dictionary should supported in NO_INVERTED_DICTIONARY
    3. Fix NullPointerException in compaction for decimal value after multiple load
   
   


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

    $ git pull https://github.com/rahulforallp/incubator-carbondata CARBONDATA-1066

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

    https://github.com/apache/carbondata/pull/927.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 #927
   
----
commit 33632d34d31ab8b101a76638f3543c8f2f576be7
Author: rahulforallp <[hidden email]>
Date:   2017-05-18T11:12:49Z

    measure shouldnot added to no_inverted_index

commit 59cf9803033754e2b9ec813b85c130c56821fe49
Author: rahulforallp <[hidden email]>
Date:   2017-05-18T11:23:49Z

    CARBONDATA-1066 supported

commit 7747f38627685d04650f1c2ec13818d0000ef9ee
Author: rahulforallp <[hidden email]>
Date:   2017-05-18T12:11:27Z

    nullpointer exception resolved for multiple load and major compaction
    some test case added

----


---
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/927
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2081/



---
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 rahulforallp commented on the issue:

    https://github.com/apache/carbondata/pull/927
 
    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/927
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2082/



---
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 kumarvishal09 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/927#discussion_r117430177
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/store/CarbonFactDataHandlerColumnar.java ---
    @@ -1366,8 +1350,9 @@ public Codec encodeAndCompressMeasures(TablePage tablePage) {
           } catch (InterruptedException e) {
             LOGGER.error(e, e.getMessage());
           }
    -      IndexStorage[] dimColumns = new IndexStorage[
    -          colGrpModel.getNoOfColumnStore() + noDictionaryCount + getExpandedComplexColsCount()];
    +      IndexStorage[] dimColumns =
    --- End diff --
   
    Some changes it is showing because of formatting , please remove


---
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 kumarvishal09 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/927#discussion_r117429668
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/ByteUtil.java ---
    @@ -66,6 +66,26 @@ public static int compare(byte[] buffer1, byte[] buffer2) {
             .compareTo(buffer1, offset1, len1, buffer2, offset2, len2);
       }
     
    +  /**
    +   * Compare method for bytes
    +   *
    +   * @param buffer1
    +   * @param buffer2
    +   * @return
    +   */
    +  public static int compareOne(byte[] buffer1, byte[] buffer2) {
    --- End diff --
   
    Remove this unused method


---
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/927
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2097/



---
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/927
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2098/



---
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/927
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2099/



---
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/927
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2100/



---
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 rahulforallp commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/927#discussion_r117447500
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/ByteUtil.java ---
    @@ -66,6 +66,26 @@ public static int compare(byte[] buffer1, byte[] buffer2) {
             .compareTo(buffer1, offset1, len1, buffer2, offset2, len2);
       }
     
    +  /**
    +   * Compare method for bytes
    +   *
    +   * @param buffer1
    +   * @param buffer2
    +   * @return
    +   */
    +  public static int compareOne(byte[] buffer1, byte[] buffer2) {
    --- End diff --
   
    @kumarvishal09 unused function has removed


---
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/927
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2102/



---
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 rahulforallp commented on the issue:

    https://github.com/apache/carbondata/pull/927
 
    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/927
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2106/



---
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 rahulforallp closed the pull request at:

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


---
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 rahulforallp reopened a pull request:

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

    [CARBONDATA-1066] Fixed NO_INVERTED_INDEX for other table properties and NPE in compaction

   
   
    1. Measure shouldn't supported inside no_inverted_index , if it is not included in sort_column or dictionary_include.
    2. Dimension excluded from dictionary should supported in NO_INVERTED_DICTIONARY
    3. Fix NullPointerException in compaction for decimal value after multiple load
   
   


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

    $ git pull https://github.com/rahulforallp/incubator-carbondata CARBONDATA-1066

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

    https://github.com/apache/carbondata/pull/927.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 #927
   
----
commit 33632d34d31ab8b101a76638f3543c8f2f576be7
Author: rahulforallp <[hidden email]>
Date:   2017-05-18T11:12:49Z

    measure shouldnot added to no_inverted_index

commit 59cf9803033754e2b9ec813b85c130c56821fe49
Author: rahulforallp <[hidden email]>
Date:   2017-05-18T11:23:49Z

    CARBONDATA-1066 supported

commit 7747f38627685d04650f1c2ec13818d0000ef9ee
Author: rahulforallp <[hidden email]>
Date:   2017-05-18T12:11:27Z

    nullpointer exception resolved for multiple load and major compaction
    some test case added

commit 5c0a1f44498489be714d89afee7a10a4bcf0c7b4
Author: rahulforallp <[hidden email]>
Date:   2017-05-19T10:00:53Z

    comment resolved

----


---
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 gvramana commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/927#discussion_r117498967
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/spark/sql/catalyst/CarbonDDLSqlParser.scala ---
    @@ -495,15 +495,36 @@ abstract class CarbonDDLSqlParser extends AbstractCarbonSparkSQLParser {
         // check duplicate columns and only 1 col left
         val distinctCols = noInvertedIdxColsProps.toSet
         // extract the no inverted index columns
    +    val dictionaryInclude = tableProperties.getOrElse(CarbonCommonConstants.DICTIONARY_INCLUDE, "")
    --- End diff --
   
    remove this validation


---
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 gvramana commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/927#discussion_r117488392
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/spark/sql/catalyst/CarbonDDLSqlParser.scala ---
    @@ -495,15 +495,36 @@ abstract class CarbonDDLSqlParser extends AbstractCarbonSparkSQLParser {
         // check duplicate columns and only 1 col left
         val distinctCols = noInvertedIdxColsProps.toSet
         // extract the no inverted index columns
    +    val dictionaryInclude = tableProperties.getOrElse(CarbonCommonConstants.DICTIONARY_INCLUDE, "")
    +      .split(",")
    +    val sortColumns = tableProperties.getOrElse(CarbonCommonConstants.SORT_COLUMNS, "").split(",")
         fields.foreach(field => {
    -      if (distinctCols.exists(x => x.equalsIgnoreCase(field.column))) {
    +      if (distinctCols.exists(x => x.equalsIgnoreCase(field.column)) &&
    +          validateColumnsForInvertedIndex(field, dictionaryInclude ++ sortColumns)) {
             noInvertedIdxCols :+= field.column
           }
         }
         )
         noInvertedIdxCols
       }
     
    +
    +  private def validateColumnsForInvertedIndex(field: Field,
    +      dictionaryIncludeOrSortColumn: Array[String]): Boolean = {
    +    val invertedIndexColumns = Array("date", "timestamp", "struct", "array")
    --- End diff --
   
    Struct, array will not have invertedindex, while data and timestamp will have invertedindex


---
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/927
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2110/



---
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 gvramana commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/927#discussion_r118670679
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/ByteUtil.java ---
    @@ -92,12 +92,12 @@ public static String convertByteToReadable(long sizeInbyte) {
         String readableSize;
         if (sizeInbyte < CarbonCommonConstants.BYTE_TO_KB_CONVERSION_FACTOR) {
           readableSize = sizeInbyte + " Byte";
    -    } else if (sizeInbyte < CarbonCommonConstants.BYTE_TO_KB_CONVERSION_FACTOR *
    -            CarbonCommonConstants.BYTE_TO_KB_CONVERSION_FACTOR) {
    +    } else if (sizeInbyte < CarbonCommonConstants.BYTE_TO_KB_CONVERSION_FACTOR
    --- End diff --
   
    take out indentation changes


---
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.
---