You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by jackylk <gi...@git.apache.org> on 2017/08/18 06:16:33 UTC
[GitHub] carbondata pull request #1265: [CARBONDATA-1128] Add direct string encoding ...
GitHub user jackylk opened a pull request:
https://github.com/apache/carbondata/pull/1265
[CARBONDATA-1128] Add direct string encoding for short string column
For short string columns less than 128 bytes, add a new encoding to improve compression and loading speed.
DirectStringCodec encode the input column by two array:
1. one for string content, stored in data page of DataChunk2
2. another for string length, stored in EncoderMeta in DataChunk2
They are compressed separately by compressor.
I have tested using TPC-H generated data (1GB)
1. For high cardinality columns (L_COMENT in LINEITEM table, distinct value is 4580667)
CREATE TABLE LINEITEM (
L_COMMENT VARCHAR(44)
)
STORED BY 'carbondata'
- Use direct string encoding
loading time: 12496 ms
size: 42M
- Use existing encoding
loading time: 12230 ms
size: 45M
2. For low cardinality columns (L_SHIPMODE in LINEITEM table, distinct value is 7)
CREATE TABLE LINEITEM (
L_SHIPMODE CHAR(10)
)
STORED BY 'carbondata'
- Use direct string encoding
loading time: 6089 ms
size: 1.4M
- Use existing encoding
loading time: 6556 ms
size: 1.7M
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jackylk/incubator-carbondata direct_string
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/1265.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 #1265
----
commit 828f108fc5c312a087f80e4470cae7666293fc3f
Author: Jacky Li <ja...@qq.com>
Date: 2017-08-17T01:57:43Z
add integral rle codec
commit 37d1c0977220eab37ba4695e0585e0045258e702
Author: Jacky Li <ja...@qq.com>
Date: 2017-08-17T13:32:02Z
decode by meta
commit 05eaa0b6ef64ba23425259006e3c57fd867f95c4
Author: Jacky Li <ja...@qq.com>
Date: 2017-08-18T06:05:38Z
add direct string codec for short string column
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/507/
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1265
Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4000/
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1265
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/24/
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add direct string encoding for sho...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Failed with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/335/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1265
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/574/
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add direct string encoding for sho...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Failed with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/275/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/543/
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/537/
---
[GitHub] carbondata pull request #1265: [CARBONDATA-1128] Add encoding for non-dictio...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1265#discussion_r137490368
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/chunk/store/ColumnPageWrapper.java ---
@@ -17,46 +17,183 @@
package org.apache.carbondata.core.datastore.chunk.store;
+import java.nio.ByteBuffer;
+import java.util.BitSet;
+
+import org.apache.carbondata.core.constants.CarbonCommonConstants;
+import org.apache.carbondata.core.datastore.ColumnType;
+import org.apache.carbondata.core.datastore.TableSpec;
import org.apache.carbondata.core.datastore.chunk.DimensionColumnDataChunk;
import org.apache.carbondata.core.datastore.page.ColumnPage;
+import org.apache.carbondata.core.keygenerator.directdictionary.DirectDictionaryGenerator;
+import org.apache.carbondata.core.metadata.datatype.DataType;
import org.apache.carbondata.core.scan.executor.infos.KeyStructureInfo;
+import org.apache.carbondata.core.scan.result.vector.CarbonColumnVector;
import org.apache.carbondata.core.scan.result.vector.ColumnVectorInfo;
+import org.apache.carbondata.core.util.ByteUtil;
+/**
+ * ColumnPage wrapper for dimension column reader
+ */
public class ColumnPageWrapper implements DimensionColumnDataChunk {
+ private TableSpec.ColumnSpec columnSpec;
private ColumnPage columnPage;
+ private int columnValueSize;
- public ColumnPageWrapper(ColumnPage columnPage) {
+ public ColumnPageWrapper(ColumnPage columnPage, int columnValueSize) {
this.columnPage = columnPage;
+ this.columnValueSize = columnValueSize;
+ this.columnSpec = columnPage.getColumnSpec();
}
@Override
public int fillChunkData(byte[] data, int offset, int columnIndex,
KeyStructureInfo restructuringInfo) {
- throw new UnsupportedOperationException("internal error");
+ int surrogate = columnPage.getInt(columnIndex);
+ ByteBuffer buffer = ByteBuffer.wrap(data);
+ buffer.putInt(offset, surrogate);
+ return columnValueSize;
}
@Override
public int fillConvertedChunkData(int rowId, int columnIndex, int[] row,
KeyStructureInfo restructuringInfo) {
- throw new UnsupportedOperationException("internal error");
+ row[columnIndex] = columnPage.getInt(rowId);
+ return columnIndex + 1;
}
@Override
public int fillConvertedChunkData(ColumnVectorInfo[] vectorInfo, int column,
KeyStructureInfo restructuringInfo) {
- throw new UnsupportedOperationException("internal error");
+ // fill the vector with data in column page
+ ColumnVectorInfo columnVectorInfo = vectorInfo[column];
+ CarbonColumnVector vector = columnVectorInfo.vector;
+ fillData(null, columnVectorInfo, vector);
+ return column + 1;
}
@Override
public int fillConvertedChunkData(int[] rowMapping, ColumnVectorInfo[] vectorInfo, int column,
KeyStructureInfo restructuringInfo) {
- throw new UnsupportedOperationException("internal error");
+ ColumnVectorInfo columnVectorInfo = vectorInfo[column];
+ CarbonColumnVector vector = columnVectorInfo.vector;
+ fillData(rowMapping, columnVectorInfo, vector);
+ return column + 1;
+ }
+
+ /**
+ * Copy columnar data from internal column page to `vector`, row information described by
+ * `columnVectorInfo`
+ */
+ private void fillData(int[] rowMapping, ColumnVectorInfo columnVectorInfo,
--- End diff --
Since all the code is handled here I think we can remove `FixedLengthDimensionDataChunk` and `VariableLengthDimensionDataChunk` and all related class hierarchy right.
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add direct string encoding for sho...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
@jackylk it seems lot of tests are failing, can you check
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata pull request #1265: [CARBONDATA-1128] Add encoding for non-dictio...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1265#discussion_r137805419
--- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelRangeGrtThanFiterExecuterImpl.java ---
@@ -366,7 +364,7 @@ private BitSet setFilterdIndexToBitSet(DimensionColumnDataChunk dimensionColumnD
BitSet bitSet = new BitSet(numerOfRows);
byte[][] filterValues = this.filterRangeValues;
// binary search can only be applied if column is sorted
- if (isNaturalSorted) {
+ if (isNaturalSorted && !dimensionColumnDataChunk.isNoDicitionaryColumn()) {
--- End diff --
In this PR, plain value (no dictionary) is using adaptive encoding, it is not sorted. I think it will make store size smaller but for some filter query it will be slower. Any suggestion?
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/512/
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add direct string encoding for sho...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Failed with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/226/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata pull request #1265: [CARBONDATA-1128] Add encoding for non-dictio...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1265#discussion_r137809952
--- Diff: core/src/main/java/org/apache/carbondata/core/datastore/chunk/store/ColumnPageWrapper.java ---
@@ -17,46 +17,183 @@
package org.apache.carbondata.core.datastore.chunk.store;
+import java.nio.ByteBuffer;
+import java.util.BitSet;
+
+import org.apache.carbondata.core.constants.CarbonCommonConstants;
+import org.apache.carbondata.core.datastore.ColumnType;
+import org.apache.carbondata.core.datastore.TableSpec;
import org.apache.carbondata.core.datastore.chunk.DimensionColumnDataChunk;
import org.apache.carbondata.core.datastore.page.ColumnPage;
+import org.apache.carbondata.core.keygenerator.directdictionary.DirectDictionaryGenerator;
+import org.apache.carbondata.core.metadata.datatype.DataType;
import org.apache.carbondata.core.scan.executor.infos.KeyStructureInfo;
+import org.apache.carbondata.core.scan.result.vector.CarbonColumnVector;
import org.apache.carbondata.core.scan.result.vector.ColumnVectorInfo;
+import org.apache.carbondata.core.util.ByteUtil;
+/**
+ * ColumnPage wrapper for dimension column reader
+ */
public class ColumnPageWrapper implements DimensionColumnDataChunk {
+ private TableSpec.ColumnSpec columnSpec;
private ColumnPage columnPage;
+ private int columnValueSize;
- public ColumnPageWrapper(ColumnPage columnPage) {
+ public ColumnPageWrapper(ColumnPage columnPage, int columnValueSize) {
this.columnPage = columnPage;
+ this.columnValueSize = columnValueSize;
+ this.columnSpec = columnPage.getColumnSpec();
}
@Override
public int fillChunkData(byte[] data, int offset, int columnIndex,
KeyStructureInfo restructuringInfo) {
- throw new UnsupportedOperationException("internal error");
+ int surrogate = columnPage.getInt(columnIndex);
+ ByteBuffer buffer = ByteBuffer.wrap(data);
+ buffer.putInt(offset, surrogate);
+ return columnValueSize;
}
@Override
public int fillConvertedChunkData(int rowId, int columnIndex, int[] row,
KeyStructureInfo restructuringInfo) {
- throw new UnsupportedOperationException("internal error");
+ row[columnIndex] = columnPage.getInt(rowId);
+ return columnIndex + 1;
}
@Override
public int fillConvertedChunkData(ColumnVectorInfo[] vectorInfo, int column,
KeyStructureInfo restructuringInfo) {
- throw new UnsupportedOperationException("internal error");
+ // fill the vector with data in column page
+ ColumnVectorInfo columnVectorInfo = vectorInfo[column];
+ CarbonColumnVector vector = columnVectorInfo.vector;
+ fillData(null, columnVectorInfo, vector);
+ return column + 1;
}
@Override
public int fillConvertedChunkData(int[] rowMapping, ColumnVectorInfo[] vectorInfo, int column,
KeyStructureInfo restructuringInfo) {
- throw new UnsupportedOperationException("internal error");
+ ColumnVectorInfo columnVectorInfo = vectorInfo[column];
+ CarbonColumnVector vector = columnVectorInfo.vector;
+ fillData(rowMapping, columnVectorInfo, vector);
+ return column + 1;
+ }
+
+ /**
+ * Copy columnar data from internal column page to `vector`, row information described by
+ * `columnVectorInfo`
+ */
+ private void fillData(int[] rowMapping, ColumnVectorInfo columnVectorInfo,
--- End diff --
For dictionary column, it still using `IndexStorageCodec` to do encoding and using `decodeDimensionLegacy` to decode, so `FixedLengthDimensionDataChunk` is still required. `VariableLengthDimensionDataChunk` is also required for backward compatiblity to read old data file
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1265
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5210/
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add direct string encoding for sho...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Failed with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/252/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/627/
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1205/
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add direct string encoding for sho...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Failed with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/233/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Failed with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/360/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/1265
retest this please
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/493/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1265
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4322/
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/518/
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Failed with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/354/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/519/
---
[GitHub] carbondata pull request #1265: [CARBONDATA-1128] Add encoding for non-dictio...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1265#discussion_r137449951
--- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelRangeLessThanEqualFilterExecuterImpl.java ---
@@ -367,7 +360,7 @@ private BitSet setFilterdIndexToBitSet(DimensionColumnDataChunk dimensionColumnD
BitSet bitSet = new BitSet(numerOfRows);
byte[][] filterValues = this.filterRangeValues;
// binary search can only be applied if column is sorted
- if (isNaturalSorted) {
+ if (isNaturalSorted && !dimensionColumnDataChunk.isNoDicitionaryColumn()) {
--- End diff --
why this check required?
---
[GitHub] carbondata pull request #1265: [CARBONDATA-1128] Add encoding for non-dictio...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk closed the pull request at:
https://github.com/apache/carbondata/pull/1265
---
[GitHub] carbondata pull request #1265: [CARBONDATA-1128] Add encoding for non-dictio...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1265#discussion_r137450241
--- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/partition/EqualToFilterImpl.java ---
@@ -50,7 +50,7 @@ public EqualToFilterImpl(EqualToExpression equalTo, PartitionInfo partitionInfo)
literal.getLiteralExpValue().toString(),
partitionInfo.getColumnSchemaList().get(0).getDataType());
if (PartitionType.RANGE == partitionInfo.getPartitionType() && value instanceof String) {
- value = ByteUtil.toBytes((String)value);
+ value = ByteUtil.toBytesForPlainValue((String)value);
--- End diff --
It seems old overloaded method looks good than this name. It is utility method so better have a overloaded method
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/494/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata pull request #1265: [CARBONDATA-1128] Add encoding for non-dictio...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1265#discussion_r137806977
--- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/partition/EqualToFilterImpl.java ---
@@ -50,7 +50,7 @@ public EqualToFilterImpl(EqualToExpression equalTo, PartitionInfo partitionInfo)
literal.getLiteralExpValue().toString(),
partitionInfo.getColumnSchemaList().get(0).getDataType());
if (PartitionType.RANGE == partitionInfo.getPartitionType() && value instanceof String) {
- value = ByteUtil.toBytes((String)value);
+ value = ByteUtil.toBytesForPlainValue((String)value);
--- End diff --
`ByteUtil.toBytes` will do `val = val ^ Integer.MIN_VALUE;`
So I think it can not be used as normal utility function to convert int to byte[4]
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add direct string encoding for sho...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Failed with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/339/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata pull request #1265: [CARBONDATA-1128] Add encoding for non-dictio...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1265#discussion_r137490913
--- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelRangeLessThanEqualFilterExecuterImpl.java ---
@@ -262,10 +259,7 @@ private BitSet getFilteredIndexes(DimensionColumnDataChunk dimensionColumnDataCh
int numerOfRows) {
byte[] defaultValue = null;
if (dimColEvaluatorInfoList.get(0).getDimension().hasEncoding(Encoding.DIRECT_DICTIONARY)) {
- DirectDictionaryGenerator directDictionaryGenerator = DirectDictionaryKeyGeneratorFactory
- .getDirectDictionaryGenerator(
- dimColEvaluatorInfoList.get(0).getDimension().getDataType());
- int key = directDictionaryGenerator.generateDirectSurrogateKey(null) + 1;
+ int key = 0;
--- End diff --
And also the null value for dictionary is 1 not 0
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/513/
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1265
Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3078/
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add encoding for non-dictionary di...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/544/
---
[GitHub] carbondata pull request #1265: [CARBONDATA-1128] Add encoding for non-dictio...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1265#discussion_r137449915
--- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelRangeLessThanEqualFilterExecuterImpl.java ---
@@ -262,10 +259,7 @@ private BitSet getFilteredIndexes(DimensionColumnDataChunk dimensionColumnDataCh
int numerOfRows) {
byte[] defaultValue = null;
if (dimColEvaluatorInfoList.get(0).getDimension().hasEncoding(Encoding.DIRECT_DICTIONARY)) {
- DirectDictionaryGenerator directDictionaryGenerator = DirectDictionaryKeyGeneratorFactory
- .getDirectDictionaryGenerator(
- dimColEvaluatorInfoList.get(0).getDimension().getDataType());
- int key = directDictionaryGenerator.generateDirectSurrogateKey(null) + 1;
+ int key = 0;
--- End diff --
I think it is better to get null key from respective interfaces instead of hard code. if the null value key changes in future also no effect here
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add direct string encoding for sho...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1265
SDV Build Failed with Spark 2.1, Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/311/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] carbondata pull request #1265: [CARBONDATA-1128] Add encoding for non-dictio...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1265#discussion_r137449599
--- Diff: core/src/main/java/org/apache/carbondata/core/scan/filter/executer/RowLevelRangeGrtThanFiterExecuterImpl.java ---
@@ -366,7 +364,7 @@ private BitSet setFilterdIndexToBitSet(DimensionColumnDataChunk dimensionColumnD
BitSet bitSet = new BitSet(numerOfRows);
byte[][] filterValues = this.filterRangeValues;
// binary search can only be applied if column is sorted
- if (isNaturalSorted) {
+ if (isNaturalSorted && !dimensionColumnDataChunk.isNoDicitionaryColumn()) {
--- End diff --
why this check is required? even no dictionary columns also can be sorted right?
---
[GitHub] carbondata issue #1265: [CARBONDATA-1128] Add direct string encoding for sho...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/1265
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---