[GitHub] carbondata pull request #1362: [CARBONDATA-1444]Support boolean

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1362#discussion_r143890810
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelFilterExecuterImpl.java ---
    @@ -203,7 +203,7 @@ private void initMeasureBlockIndexes() {
           } else {
             // specific for restructure case where default values need to be filled
             pageNumbers = blockChunkHolder.getDataBlock().numberOfPages();
    -        numberOfRows = new int[] { blockChunkHolder.getDataBlock().nodeSize() };
    +        numberOfRows = new int[] { blockChunkHolder.getDataBlock().nodeSize()};
    --- End diff --
   
    Ok.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1362#discussion_r143890823
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelFilterExecuterImpl.java ---
    @@ -215,7 +215,7 @@ private void initMeasureBlockIndexes() {
           } else {
             // specific for restructure case where default values need to be filled
             pageNumbers = blockChunkHolder.getDataBlock().numberOfPages();
    -        numberOfRows = new int[] { blockChunkHolder.getDataBlock().nodeSize() };
    +        numberOfRows = new int[] { blockChunkHolder.getDataBlock().nodeSize()};
    --- End diff --
   
    Ok.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1362#discussion_r143890833
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/bool/BooleanConvert.java ---
    @@ -0,0 +1,63 @@
    +/*
    + * 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.carbondata.core.datastore.page.encoding.bool;
    +
    +/**
    + * convert tools for boolean data type
    + */
    +public class BooleanConvert {
    +
    +  public static final byte trueValue = 1;
    +  public static final byte falseValue = 0;
    +  /**
    +   * convert boolean to byte
    +   *
    +   * @param data data of boolean data type
    +   * @return byte type data by convert
    +   */
    +  public static byte boolean2Byte(boolean data) {
    +    return data ? (byte) 1 : (byte) 0;
    +  }
    +
    +  /**
    +   * convert byte to boolean
    +   *
    +   * @param data byte type data
    +   * @return boolean type data
    +   */
    +  public static boolean byte2Boolean(int data) {
    +    return data == 1 ? true : false;
    --- End diff --
   
    Ok.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1362#discussion_r143890846
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/bool/BooleanConvert.java ---
    @@ -0,0 +1,63 @@
    +/*
    + * 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.carbondata.core.datastore.page.encoding.bool;
    +
    +/**
    + * convert tools for boolean data type
    + */
    +public class BooleanConvert {
    +
    +  public static final byte trueValue = 1;
    +  public static final byte falseValue = 0;
    +  /**
    +   * convert boolean to byte
    +   *
    +   * @param data data of boolean data type
    +   * @return byte type data by convert
    +   */
    +  public static byte boolean2Byte(boolean data) {
    +    return data ? (byte) 1 : (byte) 0;
    --- End diff --
   
    Ok.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1362#discussion_r143890886
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/store/TablePage.java ---
    @@ -187,6 +188,12 @@ private void convertToColumnarAndAddToPages(int rowId, CarbonRow row, byte[] mdk
               value != null) {
             value = ((Decimal) value).toJavaBigDecimal();
           }
    +      if (measurePages[i].getColumnSpec().getSchemaDataType()
    +              == DataType.BOOLEAN && value != null) {
    --- End diff --
   
    Ok, I have changed it.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1362#discussion_r143890910
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/page/ColumnPage.java ---
    @@ -361,6 +360,10 @@ public void putData(int rowId, Object value) {
           return;
         }
         switch (dataType) {
    +      case BOOLEAN:
    +        putByte(rowId, (byte) value);
    --- End diff --
   
    I have deleted it.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1362#discussion_r143891060
 
    --- Diff: integration/spark2/src/test/scala/org/apache/carbondata/spark/testsuite/booleantype/BooleanDataTypesLoadTest.scala ---
    @@ -0,0 +1,655 @@
    +/*
    + * 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.carbondata.spark.testsuite.booleantype
    +
    +import java.io.File
    +
    +import org.apache.spark.sql.Row
    +import org.apache.spark.sql.test.util.QueryTest
    +import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
    +
    +/**
    +  * Created by root on 9/17/17.
    +  */
    +class BooleanDataTypesLoadTest extends QueryTest with BeforeAndAfterEach with BeforeAndAfterAll {
    +  val rootPath = new File(this.getClass.getResource("/").getPath
    +    + "../../../..").getCanonicalPath
    +
    --- End diff --
   
    OK, I add  some testcase to use unsafe in BooleanDataTypesLoadTest.scala and BooleanDataTypesBigFileTest.scala


---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/1362
 
    @jackylk  I have added some descriptions for test cases in this PR.


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1362
 
    Build Success with Spark 1.6, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/308/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1362
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/433/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/1362
 
    @jackylk The conflicts have been solved


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1362
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/454/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1362
 
    Build Failed with Spark 1.6, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/329/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1362
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1082/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1362
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/455/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1362
 
    Build Success with Spark 1.6, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/330/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/1362
 
    Conflicts has been fixed， please review it @jackylk


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1362
 
    Build Failed with Spark 1.6, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/368/



---