[GitHub] [carbondata] kumarvishal09 commented on pull request #3887: [CARBONDATA-3830] Support Array and Struct of all primitive type reading from presto
kumarvishal09 commented on pull request #3887: URL: https://github.com/apache/carbondata/pull/3887#issuecomment-683368433 LGTM This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3819: [CARBONDATA-3855]support carbon SDK to load data from different files
CarbonDataQA1 commented on pull request #3819: URL: https://github.com/apache/carbondata/pull/3819#issuecomment-683356210 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3916/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3819: [CARBONDATA-3855]support carbon SDK to load data from different files
CarbonDataQA1 commented on pull request #3819: URL: https://github.com/apache/carbondata/pull/3819#issuecomment-683356104 Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2175/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3887: [CARBONDATA-3830] Support Array and Struct of all primitive type reading from presto
CarbonDataQA1 commented on pull request #3887: URL: https://github.com/apache/carbondata/pull/3887#issuecomment-683321068 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3915/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3887: [CARBONDATA-3830] Support Array and Struct of all primitive type reading from presto
CarbonDataQA1 commented on pull request #3887: URL: https://github.com/apache/carbondata/pull/3887#issuecomment-683320513 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2174/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3887: [CARBONDATA-3830] Support Array and Struct of all primitive type reading from presto
ajantha-bhat commented on a change in pull request #3887: URL: https://github.com/apache/carbondata/pull/3887#discussion_r479661551 ## File path: integration/presto/src/main/prestosql/org/apache/carbondata/presto/readers/ComplexTypeStreamReader.java ## @@ -0,0 +1,196 @@ +/* + * 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.presto.readers; + +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.Optional; + +import io.prestosql.spi.block.ArrayBlock; +import io.prestosql.spi.block.RowBlock; +import io.prestosql.spi.type.*; + +import org.apache.carbondata.core.metadata.datatype.DataType; +import org.apache.carbondata.core.metadata.datatype.DataTypes; +import org.apache.carbondata.core.metadata.datatype.StructField; +import org.apache.carbondata.core.scan.result.vector.CarbonColumnVector; +import org.apache.carbondata.core.scan.result.vector.impl.CarbonColumnVectorImpl; + +import io.prestosql.spi.block.Block; +import io.prestosql.spi.block.BlockBuilder; + +import org.apache.carbondata.presto.CarbonVectorBatch; +import org.apache.carbondata.presto.ColumnarVectorWrapperDirect; + +/** + * Class to read the complex type Stream [array/struct/map] + */ + +public class ComplexTypeStreamReader extends CarbonColumnVectorImpl +implements PrestoVectorBlockBuilder { + + protected int batchSize; + + protected Type type; + protected BlockBuilder builder; + + public ComplexTypeStreamReader(int batchSize, StructField field) { +super(batchSize, field.getDataType()); +this.batchSize = batchSize; +this.type = getType(field); +ArrayList childrenList = new ArrayList<>(); Review comment: check and handled all the newly added code This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3887: [CARBONDATA-3830] Support Array and Struct of all primitive type reading from presto
ajantha-bhat commented on a change in pull request #3887: URL: https://github.com/apache/carbondata/pull/3887#discussion_r479661505 ## File path: integration/presto/src/main/prestosql/org/apache/carbondata/presto/readers/ComplexTypeStreamReader.java ## @@ -0,0 +1,196 @@ +/* + * 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.presto.readers; + +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.Optional; + +import io.prestosql.spi.block.ArrayBlock; +import io.prestosql.spi.block.RowBlock; +import io.prestosql.spi.type.*; + +import org.apache.carbondata.core.metadata.datatype.DataType; +import org.apache.carbondata.core.metadata.datatype.DataTypes; +import org.apache.carbondata.core.metadata.datatype.StructField; +import org.apache.carbondata.core.scan.result.vector.CarbonColumnVector; +import org.apache.carbondata.core.scan.result.vector.impl.CarbonColumnVectorImpl; + +import io.prestosql.spi.block.Block; +import io.prestosql.spi.block.BlockBuilder; + +import org.apache.carbondata.presto.CarbonVectorBatch; +import org.apache.carbondata.presto.ColumnarVectorWrapperDirect; + +/** + * Class to read the complex type Stream [array/struct/map] + */ + +public class ComplexTypeStreamReader extends CarbonColumnVectorImpl +implements PrestoVectorBlockBuilder { + + protected int batchSize; + + protected Type type; + protected BlockBuilder builder; + + public ComplexTypeStreamReader(int batchSize, StructField field) { +super(batchSize, field.getDataType()); +this.batchSize = batchSize; +this.type = getType(field); +ArrayList childrenList = new ArrayList<>(); Review comment: done ## File path: core/src/main/java/org/apache/carbondata/core/metadata/datatype/DecimalConverterFactory.java ## @@ -187,6 +197,43 @@ public void fillVector(Object valuesToBeConverted, int size, vector.putDecimal(i, value, precision); } } + } else if (pageType == DataTypes.BYTE_ARRAY) { +// complex primitive decimal dimension +int offset = 0; +for (int j = 0; j < size; j++) { + // here decimal data will be Length[4 byte], scale[1 byte], value[Length byte] + int len = ByteBuffer.wrap(data, offset, DataTypes.INT.getSizeInBytes()).getInt(); + offset += DataTypes.INT.getSizeInBytes(); Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3887: [CARBONDATA-3830] Support Array and Struct of all primitive type reading from presto
ajantha-bhat commented on a change in pull request #3887: URL: https://github.com/apache/carbondata/pull/3887#discussion_r479661536 ## File path: core/src/main/java/org/apache/carbondata/core/metadata/datatype/DecimalConverterFactory.java ## @@ -187,6 +197,43 @@ public void fillVector(Object valuesToBeConverted, int size, vector.putDecimal(i, value, precision); } } + } else if (pageType == DataTypes.BYTE_ARRAY) { +// complex primitive decimal dimension +int offset = 0; +for (int j = 0; j < size; j++) { + // here decimal data will be Length[4 byte], scale[1 byte], value[Length byte] + int len = ByteBuffer.wrap(data, offset, DataTypes.INT.getSizeInBytes()).getInt(); + offset += DataTypes.INT.getSizeInBytes(); Review comment: handled all the 5 files including base code This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3861: [CARBONDATA-3922] Support order by limit push down for secondary index queries
CarbonDataQA1 commented on pull request #3861: URL: https://github.com/apache/carbondata/pull/3861#issuecomment-683306685 Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3913/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3861: [CARBONDATA-3922] Support order by limit push down for secondary index queries
CarbonDataQA1 commented on pull request #3861: URL: https://github.com/apache/carbondata/pull/3861#issuecomment-683306326 Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2172/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] kumarvishal09 commented on a change in pull request #3887: [CARBONDATA-3830] Support Array and Struct of all primitive type reading from presto
kumarvishal09 commented on a change in pull request #3887: URL: https://github.com/apache/carbondata/pull/3887#discussion_r479657086 ## File path: integration/presto/src/main/prestosql/org/apache/carbondata/presto/readers/ComplexTypeStreamReader.java ## @@ -0,0 +1,196 @@ +/* + * 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.presto.readers; + +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.Optional; + +import io.prestosql.spi.block.ArrayBlock; +import io.prestosql.spi.block.RowBlock; +import io.prestosql.spi.type.*; + +import org.apache.carbondata.core.metadata.datatype.DataType; +import org.apache.carbondata.core.metadata.datatype.DataTypes; +import org.apache.carbondata.core.metadata.datatype.StructField; +import org.apache.carbondata.core.scan.result.vector.CarbonColumnVector; +import org.apache.carbondata.core.scan.result.vector.impl.CarbonColumnVectorImpl; + +import io.prestosql.spi.block.Block; +import io.prestosql.spi.block.BlockBuilder; + +import org.apache.carbondata.presto.CarbonVectorBatch; +import org.apache.carbondata.presto.ColumnarVectorWrapperDirect; + +/** + * Class to read the complex type Stream [array/struct/map] + */ + +public class ComplexTypeStreamReader extends CarbonColumnVectorImpl +implements PrestoVectorBlockBuilder { + + protected int batchSize; + + protected Type type; + protected BlockBuilder builder; + + public ComplexTypeStreamReader(int batchSize, StructField field) { +super(batchSize, field.getDataType()); +this.batchSize = batchSize; +this.type = getType(field); +ArrayList childrenList = new ArrayList<>(); Review comment: Better use Interface type, List instead of ArrayList This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] kumarvishal09 commented on a change in pull request #3887: [CARBONDATA-3830] Support Array and Struct of all primitive type reading from presto
kumarvishal09 commented on a change in pull request #3887: URL: https://github.com/apache/carbondata/pull/3887#discussion_r479656884 ## File path: core/src/main/java/org/apache/carbondata/core/metadata/datatype/DecimalConverterFactory.java ## @@ -187,6 +197,43 @@ public void fillVector(Object valuesToBeConverted, int size, vector.putDecimal(i, value, precision); } } + } else if (pageType == DataTypes.BYTE_ARRAY) { +// complex primitive decimal dimension +int offset = 0; +for (int j = 0; j < size; j++) { + // here decimal data will be Length[4 byte], scale[1 byte], value[Length byte] + int len = ByteBuffer.wrap(data, offset, DataTypes.INT.getSizeInBytes()).getInt(); + offset += DataTypes.INT.getSizeInBytes(); Review comment: DataTypes.INT.getSizeInBytes), It’s better to assign to some variable and use it inside loop, instead calling method every time This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3887: [CARBONDATA-3830] Support Array and Struct of all primitive type reading from presto
ajantha-bhat commented on a change in pull request #3887: URL: https://github.com/apache/carbondata/pull/3887#discussion_r479656578 ## File path: core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/adaptive/AdaptiveDeltaFloatingCodec.java ## @@ -282,6 +288,12 @@ public void decodeAndFillVector(byte[] pageData, ColumnVectorInfo vectorInfo, Bi for (int i = 0; i < size; i += DataTypes.INT.getSizeInBytes()) { vector.putFloat(rowId++, (max - ByteUtil.toIntLittleEndian(pageData, i)) / floatFactor); } +} else if (pageDataType == DataTypes.LONG) { Review comment: jira is created https://issues.apache.org/jira/browse/CARBONDATA-3965 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Created] (CARBONDATA-3965) Adaptive encoding of Complex primitive float is using log value to store float (4 bytes) data
Ajantha Bhat created CARBONDATA-3965: Summary: Adaptive encoding of Complex primitive float is using log value to store float (4 bytes) data Key: CARBONDATA-3965 URL: https://issues.apache.org/jira/browse/CARBONDATA-3965 Project: CarbonData Issue Type: Bug Reporter: Ajantha Bhat I have tested, With current UT itself it is hitting. for [Null, 5.512] it is using long as storage for complex primitive adaptive. Base behavior needs to check. I guess it can be analyzed separately For this, I have checked If No complex type, (if it is just primitive type) same values goes to DirectCompress, not adaptive. But for complex primitive it goes to adaptive because of below code. And as min max is stored as double precision. Long is chosen for this. {{DefaultEncodingFactory#selectCodecByAlgorithmForFloating()}} {{} else if (decimalCount < 0 && !isComplexPrimitive) \{ return new DirectCompressCodec(DataTypes.DOUBLE); } else \{ return getColumnPageCodec(stats, isComplexPrimitive, columnSpec, srcDataType, maxValue, minValue, decimalCount, absMaxValue); }}} {{}} I don't know (remember) why complex primitive should not enter direct compress. why that check is explicitly added.{{}} -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [carbondata] kumarvishal09 commented on a change in pull request #3887: [CARBONDATA-3830] Support Array and Struct of all primitive type reading from presto
kumarvishal09 commented on a change in pull request #3887: URL: https://github.com/apache/carbondata/pull/3887#discussion_r479656097 ## File path: core/src/main/java/org/apache/carbondata/core/datastore/page/encoding/adaptive/AdaptiveDeltaFloatingCodec.java ## @@ -282,6 +288,12 @@ public void decodeAndFillVector(byte[] pageData, ColumnVectorInfo vectorInfo, Bi for (int i = 0; i < size; i += DataTypes.INT.getSizeInBytes()) { vector.putFloat(rowId++, (max - ByteUtil.toIntLittleEndian(pageData, i)) / floatFactor); } +} else if (pageDataType == DataTypes.LONG) { Review comment: Can u pls raise a jira for above issue This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] ajantha-bhat commented on pull request #3861: [CARBONDATA-3922] Support order by limit push down for secondary index queries
ajantha-bhat commented on pull request #3861: URL: https://github.com/apache/carbondata/pull/3861#issuecomment-683291782 @QiangCai , @akashrn5 @kunal642 : please check and merge This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3861: [CARBONDATA-3922] Support order by limit push down for secondary index queries
CarbonDataQA1 commented on pull request #3861: URL: https://github.com/apache/carbondata/pull/3861#issuecomment-683282976 Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2171/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3861: [CARBONDATA-3922] Support order by limit push down for secondary index queries
CarbonDataQA1 commented on pull request #3861: URL: https://github.com/apache/carbondata/pull/3861#issuecomment-683282750 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3912/ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org