[GitHub] carbondata pull request #2901: [CARBONDATA-3081] Fixed NPE for boolean type ...
Github user asfgit closed the pull request at: https://github.com/apache/carbondata/pull/2901 ---
[GitHub] carbondata pull request #2901: [CARBONDATA-3081] Fixed NPE for boolean type ...
Github user kunal642 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2901#discussion_r231001835
--- Diff:
hadoop/src/main/java/org/apache/carbondata/hadoop/util/CarbonVectorizedRecordReader.java
---
@@ -171,13 +171,20 @@ public Object getCurrentValue() throws IOException,
InterruptedException {
rowCount += 1;
Object[] row = new Object[carbonColumnarBatch.columnVectors.length];
for (int i = 0; i < carbonColumnarBatch.columnVectors.length; i ++) {
+ Object data = carbonColumnarBatch.columnVectors[i].getData(batchIdx
- 1);
if (carbonColumnarBatch.columnVectors[i].getType() ==
DataTypes.STRING
|| carbonColumnarBatch.columnVectors[i].getType() ==
DataTypes.VARCHAR) {
-byte[] data = (byte[])
carbonColumnarBatch.columnVectors[i].getData(batchIdx - 1);
-row[i] = ByteUtil.toString(data, 0, data.length);
+if (data == null) {
+ row[i] = null;
+} else {
+ row[i] = ByteUtil.toString((byte[]) data, 0, (((byte[])
data).length));
+}
} else if (carbonColumnarBatch.columnVectors[i].getType() ==
DataTypes.BOOLEAN) {
-byte data = (byte)
carbonColumnarBatch.columnVectors[i].getData(batchIdx - 1);
-row[i] = ByteUtil.toBoolean(data);
+if (data == null) {
+ row[i] = null;
+} else {
+ row[i] = ByteUtil.toBoolean((byte) data);
+}
--- End diff --
getData is already has a check for null values. Because here explicit
conversion is required therefore null check had to be added.
---
[GitHub] carbondata pull request #2901: [CARBONDATA-3081] Fixed NPE for boolean type ...
Github user kunal642 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2901#discussion_r231001760
--- Diff:
store/sdk/src/test/java/org/apache/carbondata/sdk/file/CarbonReaderTest.java ---
@@ -1844,4 +1844,53 @@ public void testVectorReader() {
}
}
+ @Test
+ public void testReadingNullValues() {
+String path = "./testWriteFiles";
+try {
+ FileUtils.deleteDirectory(new File(path));
+
+ Field[] fields = new Field[2];
+ fields[0] = new Field("stringField", DataTypes.STRING);
+ fields[1] = new Field("shortField", DataTypes.BOOLEAN);
--- End diff --
done
---
[GitHub] carbondata pull request #2901: [CARBONDATA-3081] Fixed NPE for boolean type ...
Github user manishgupta88 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2901#discussion_r230818772
--- Diff:
hadoop/src/main/java/org/apache/carbondata/hadoop/util/CarbonVectorizedRecordReader.java
---
@@ -171,13 +171,20 @@ public Object getCurrentValue() throws IOException,
InterruptedException {
rowCount += 1;
Object[] row = new Object[carbonColumnarBatch.columnVectors.length];
for (int i = 0; i < carbonColumnarBatch.columnVectors.length; i ++) {
+ Object data = carbonColumnarBatch.columnVectors[i].getData(batchIdx
- 1);
if (carbonColumnarBatch.columnVectors[i].getType() ==
DataTypes.STRING
|| carbonColumnarBatch.columnVectors[i].getType() ==
DataTypes.VARCHAR) {
-byte[] data = (byte[])
carbonColumnarBatch.columnVectors[i].getData(batchIdx - 1);
-row[i] = ByteUtil.toString(data, 0, data.length);
+if (data == null) {
+ row[i] = null;
+} else {
+ row[i] = ByteUtil.toString((byte[]) data, 0, (((byte[])
data).length));
+}
} else if (carbonColumnarBatch.columnVectors[i].getType() ==
DataTypes.BOOLEAN) {
-byte data = (byte)
carbonColumnarBatch.columnVectors[i].getData(batchIdx - 1);
-row[i] = ByteUtil.toBoolean(data);
+if (data == null) {
+ row[i] = null;
+} else {
+ row[i] = ByteUtil.toBoolean((byte) data);
+}
--- End diff --
For other dataTypes is the same handling of null required?...If required
then you can move the if check for `data == null` before first if check and set
the row to null if data is null and continue
---
[GitHub] carbondata pull request #2901: [CARBONDATA-3081] Fixed NPE for boolean type ...
Github user manishgupta88 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2901#discussion_r230816465
--- Diff:
store/sdk/src/test/java/org/apache/carbondata/sdk/file/CarbonReaderTest.java ---
@@ -1844,4 +1844,53 @@ public void testVectorReader() {
}
}
+ @Test
+ public void testReadingNullValues() {
+String path = "./testWriteFiles";
+try {
+ FileUtils.deleteDirectory(new File(path));
+
+ Field[] fields = new Field[2];
+ fields[0] = new Field("stringField", DataTypes.STRING);
+ fields[1] = new Field("shortField", DataTypes.BOOLEAN);
--- End diff --
Rename `shortField` to `booleanField`
---
[GitHub] carbondata pull request #2901: [CARBONDATA-3081] Fixed NPE for boolean type ...
GitHub user kunal642 opened a pull request: https://github.com/apache/carbondata/pull/2901 [CARBONDATA-3081] Fixed NPE for boolean type column with null value Be sure to do all of the following checklist to help us incorporate your contribution quickly and easily: - [ ] Any interfaces changed? - [ ] Any backward compatibility impacted? - [ ] Document update required? - [ ] Testing done Please provide details on - Whether new unit test cases have been added or why no new tests are required? - How it is tested? Please attach test report. - Is it a performance related change? Please attach the performance test report. - Any additional information to help reviewers in testing this change. - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA. You can merge this pull request into a Git repository by running: $ git pull https://github.com/kunal642/carbondata npe_fix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/carbondata/pull/2901.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 #2901 commit 906eee74f20fbc1a9983db1ce0073356649a7724 Author: kunal642 Date: 2018-11-05T13:16:44Z fixed NPE for boolean type column with null value ---
