You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@carbondata.apache.org by ja...@apache.org on 2019/09/22 15:16:58 UTC
[carbondata] branch master updated: [HOTFIX] Fix wrong min/max
index of measure
This is an automated email from the ASF dual-hosted git repository.
jackylk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/carbondata.git
The following commit(s) were added to refs/heads/master by this push:
new 5bab524 [HOTFIX] Fix wrong min/max index of measure
5bab524 is described below
commit 5bab5244e7b7a90fd6a660ff359a2da3321eb081
Author: QiangCai <qi...@qq.com>
AuthorDate: Wed Sep 18 21:02:15 2019 +0800
[HOTFIX] Fix wrong min/max index of measure
This closes #3394
---
.../carbondata/core/util/CarbonMetadataUtil.java | 64 ++++----
.../org/apache/carbondata/sdk/file/MinMaxTest.java | 161 +++++++++++++++++++++
2 files changed, 188 insertions(+), 37 deletions(-)
diff --git a/core/src/main/java/org/apache/carbondata/core/util/CarbonMetadataUtil.java b/core/src/main/java/org/apache/carbondata/core/util/CarbonMetadataUtil.java
index f35afc0..7414ab7 100644
--- a/core/src/main/java/org/apache/carbondata/core/util/CarbonMetadataUtil.java
+++ b/core/src/main/java/org/apache/carbondata/core/util/CarbonMetadataUtil.java
@@ -477,50 +477,40 @@ public class CarbonMetadataUtil {
ByteBuffer firstBuffer = null;
ByteBuffer secondBuffer = null;
if (dataType == DataTypes.BOOLEAN || dataType == DataTypes.BYTE) {
- return first[0] - second[0];
+ if (first[0] > second[0]) {
+ return 1;
+ } else if (first[0] < second[0]) {
+ return -1;
+ }
+ return 0;
} else if (dataType == DataTypes.DOUBLE) {
- firstBuffer = ByteBuffer.allocate(8);
- firstBuffer.put(first);
- secondBuffer = ByteBuffer.allocate(8);
- secondBuffer.put(second);
- firstBuffer.flip();
- secondBuffer.flip();
- double compare = firstBuffer.getDouble() - secondBuffer.getDouble();
- if (compare > 0) {
- compare = 1;
- } else if (compare < 0) {
- compare = -1;
+ double firstValue = ((ByteBuffer) (ByteBuffer.allocate(8).put(first).flip())).getDouble();
+ double secondValue = ((ByteBuffer) (ByteBuffer.allocate(8).put(second).flip())).getDouble();
+ if (firstValue > secondValue) {
+ return 1;
+ } else if (firstValue < secondValue) {
+ return -1;
}
- return (int) compare;
+ return 0;
} else if (dataType == DataTypes.FLOAT) {
- firstBuffer = ByteBuffer.allocate(8);
- firstBuffer.put(first);
- secondBuffer = ByteBuffer.allocate(8);
- secondBuffer.put(second);
- firstBuffer.flip();
- secondBuffer.flip();
- double compare = firstBuffer.getFloat() - secondBuffer.getFloat();
- if (compare > 0) {
- compare = 1;
- } else if (compare < 0) {
- compare = -1;
+ float firstValue = ((ByteBuffer) (ByteBuffer.allocate(8).put(first).flip())).getFloat();
+ float secondValue = ((ByteBuffer) (ByteBuffer.allocate(8).put(second).flip())).getFloat();
+ if (firstValue > secondValue) {
+ return 1;
+ } else if (firstValue < secondValue) {
+ return -1;
}
- return (int) compare;
+ return 0;
} else if (dataType == DataTypes.LONG || dataType == DataTypes.INT
|| dataType == DataTypes.SHORT) {
- firstBuffer = ByteBuffer.allocate(8);
- firstBuffer.put(first);
- secondBuffer = ByteBuffer.allocate(8);
- secondBuffer.put(second);
- firstBuffer.flip();
- secondBuffer.flip();
- long compare = firstBuffer.getLong() - secondBuffer.getLong();
- if (compare > 0) {
- compare = 1;
- } else if (compare < 0) {
- compare = -1;
+ long firstValue = ((ByteBuffer) (ByteBuffer.allocate(8).put(first).flip())).getLong();
+ long secondValue = ((ByteBuffer) (ByteBuffer.allocate(8).put(second).flip())).getLong();
+ if (firstValue > secondValue) {
+ return 1;
+ } else if (firstValue < secondValue) {
+ return -1;
}
- return (int) compare;
+ return 0;
} else if (DataTypes.isDecimal(dataType)) {
return DataTypeUtil.byteToBigDecimal(first).compareTo(DataTypeUtil.byteToBigDecimal(second));
} else {
diff --git a/store/sdk/src/test/java/org/apache/carbondata/sdk/file/MinMaxTest.java b/store/sdk/src/test/java/org/apache/carbondata/sdk/file/MinMaxTest.java
new file mode 100644
index 0000000..c26fdd5
--- /dev/null
+++ b/store/sdk/src/test/java/org/apache/carbondata/sdk/file/MinMaxTest.java
@@ -0,0 +1,161 @@
+/*
+ * 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.sdk.file;
+
+import java.io.File;
+import java.io.FileFilter;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.List;
+
+import org.apache.carbondata.core.constants.CarbonCommonConstants;
+import org.apache.carbondata.core.metadata.datatype.DataTypes;
+import org.apache.carbondata.core.reader.CarbonIndexFileReader;
+import org.apache.carbondata.format.BlockIndex;
+
+import org.apache.commons.io.FileUtils;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * MinMaxTest
+ */
+public class MinMaxTest {
+ String path = "./testMinMax";
+ @Before
+ public void mkDir() throws IOException {
+ File file = new File(path);
+ FileUtils.deleteDirectory(file);
+ FileUtils.forceMkdir(file);
+ }
+
+ @After
+ public void deleteDir() throws IOException {
+ FileUtils.deleteDirectory(new File(path));
+ }
+
+ @Test
+ public void testMinMax() throws IOException {
+ Field[] fields = new Field[6];
+ fields[0] = new Field("byteField", DataTypes.BYTE);
+ fields[1] = new Field("shortField", DataTypes.SHORT);
+ fields[2] = new Field("infField", DataTypes.INT);
+ fields[3] = new Field("longField", DataTypes.LONG);
+ fields[4] = new Field("floatField", DataTypes.FLOAT);
+ fields[5] = new Field("doubleField", DataTypes.DOUBLE);
+ writeFilesAndVerify(new Schema(fields), path);
+ readIndexAndVerify(path);
+ }
+
+ public static void writeFilesAndVerify(Schema schema, String path) {
+ try {
+ CarbonWriter writer =
+ CarbonWriter
+ .builder()
+ .outputPath(path)
+ .withCsvInput(schema)
+ .writtenBy("TestUtil")
+ .build();
+ String[] row1 = new String[] {
+ String.valueOf(Byte.MIN_VALUE),
+ String.valueOf(Short.MIN_VALUE),
+ String.valueOf(Integer.MIN_VALUE),
+ String.valueOf(Long.MIN_VALUE),
+ String.valueOf(Float.MIN_VALUE),
+ String.valueOf(Double.MIN_VALUE)
+ };
+ for (int i = 0; i < 10; i++) {
+ writer.write(row1);
+ }
+ String[] row2 = new String[] {
+ String.valueOf(Byte.MAX_VALUE),
+ String.valueOf(Short.MAX_VALUE),
+ String.valueOf(Integer.MAX_VALUE),
+ String.valueOf(Long.MAX_VALUE),
+ String.valueOf(Float.MAX_VALUE),
+ String.valueOf(Double.MAX_VALUE)
+ };
+ for (int i = 0; i < 26664; i++) {
+ writer.write(row2);
+ }
+ String[] row3 = new String[] {
+ String.valueOf(Byte.MAX_VALUE - 10),
+ String.valueOf(Short.MAX_VALUE - 10),
+ String.valueOf(Integer.MAX_VALUE - 10),
+ String.valueOf(Long.MAX_VALUE - 10),
+ String.valueOf(Float.MAX_VALUE - 10),
+ String.valueOf(Double.MAX_VALUE - 10)
+ };
+ for (int i = 0; i < 6666; i++) {
+ writer.write(row3);
+ }
+ writer.close();
+ } catch (Exception e) {
+ e.printStackTrace();
+ Assert.assertTrue(false);
+ }
+ File[] dataFiles = new File(path).listFiles(new FileFilter() {
+ @Override public boolean accept(File pathname) {
+ return pathname.getName().endsWith(CarbonCommonConstants.FACT_FILE_EXT);
+ }
+ });
+ Assert.assertTrue(dataFiles != null);
+ Assert.assertTrue(dataFiles.length > 0);
+ }
+
+ public static void readIndexAndVerify(String path) {
+ File[] dataFiles = new File(path).listFiles(new FileFilter() {
+ @Override public boolean accept(File pathname) {
+ return pathname.getName().endsWith(CarbonCommonConstants.UPDATE_INDEX_FILE_EXT);
+ }
+ });
+ Assert.assertTrue(dataFiles != null);
+ Assert.assertTrue(dataFiles.length > 0);
+ CarbonIndexFileReader reader = new CarbonIndexFileReader();
+ try {
+ reader.openThriftReader(dataFiles[0].getCanonicalPath());
+ reader.readIndexHeader();
+ BlockIndex index = reader.readBlockIndexInfo();
+ List<ByteBuffer> maxValues = index.block_index.min_max_index.max_values;
+ Assert.assertEquals(Byte.MAX_VALUE, maxValues.get(0).get());
+ maxValues.get(1).position(6);
+ Assert.assertEquals(Short.MAX_VALUE, maxValues.get(1).getShort());
+ maxValues.get(2).position(4);
+ Assert.assertEquals(Integer.MAX_VALUE, maxValues.get(2).getInt());
+ Assert.assertEquals(Long.MAX_VALUE, maxValues.get(3).getLong());
+ Assert.assertTrue(Float.MAX_VALUE - maxValues.get(4).getFloat() < 1);
+ Assert.assertTrue(Double.MAX_VALUE - maxValues.get(5).getDouble() < 1);
+ List<ByteBuffer> minValues = index.block_index.min_max_index.min_values;
+ Assert.assertEquals(Byte.MIN_VALUE, minValues.get(0).get());
+ minValues.get(1).position(6);
+ Assert.assertEquals(Short.MIN_VALUE, minValues.get(1).getShort());
+ minValues.get(2).position(4);
+ Assert.assertEquals(Integer.MIN_VALUE, minValues.get(2).getInt());
+ Assert.assertEquals(Long.MIN_VALUE, minValues.get(3).getLong());
+ Assert.assertTrue(minValues.get(4).getFloat() - Float.MIN_VALUE < 1);
+ Assert.assertTrue(minValues.get(5).getDouble() - Double.MIN_VALUE < 1);
+ } catch (IOException e) {
+ e.printStackTrace();
+ Assert.assertTrue(false);
+ } finally {
+ reader.closeThriftReader();
+ }
+ }
+}