You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "parthchandra (via GitHub)" <gi...@apache.org> on 2025/01/14 04:32:18 UTC
[PR] fix: handle loading of complex types into CometVector correctly in iceberg_compat scans [datafusion-comet]
parthchandra opened a new pull request, #1279:
URL: https://github.com/apache/datafusion-comet/pull/1279
NativeColumnReader was reading all types into a CometPlainVector which is correct only for primitive types. The PR updates the code to use NativeUtil and follow the same logic as CometExecIterator.
This addresses 12 unit test failures involving ColumnarShuffle on struct types.
--
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.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: handle loading of complex types into CometVector correctly in iceberg_compat scans [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on PR #1279:
URL: https://github.com/apache/datafusion-comet/pull/1279#issuecomment-2588927595
The PR also replaces the use of ColumnDescriptor (again valid only for primitive types) with parquet.schema.Type in NativeBatchReader
--
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.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: handle loading of complex types into CometVector correctly in iceberg_compat scans [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove merged PR #1279:
URL: https://github.com/apache/datafusion-comet/pull/1279
--
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.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: handle loading of complex types into CometVector correctly in iceberg_compat scans [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on code in PR #1279:
URL: https://github.com/apache/datafusion-comet/pull/1279#discussion_r1914230645
##########
common/src/main/java/org/apache/comet/parquet/NativeColumnReader.java:
##########
@@ -144,47 +141,19 @@ public CometDecodedVector loadVector() {
Native.currentColumnBatch(nativeBatchHandle, columnNum, arrayAddr, schemaAddr);
- FieldVector vector = importer.importVector(array, schema);
+ ArrowArray[] arrays = {array};
+ ArrowSchema[] schemas = {schema};
- DictionaryEncoding dictionaryEncoding = vector.getField().getDictionary();
-
- CometPlainVector cometVector = new CometPlainVector(vector, useDecimal128);
+ CometDecodedVector cometVector =
+ (CometDecodedVector)
+ scala.collection.JavaConverters.seqAsJavaList(nativeUtil.importVector(arrays, schemas))
+ .get(0);
// Update whether the current vector contains any null values. This is used in the following
// batch(s) to determine whether we can skip loading the native vector.
hadNull = cometVector.hasNull();
- if (dictionaryEncoding == null) {
- if (dictionary != null) {
- // This means the column was using dictionary encoding but now has fall-back to plain
- // encoding, on the native side. Setting 'dictionary' to null here, so we can use it as
- // a condition to check if we can re-use vector later.
- dictionary = null;
- }
- // Either the column is not dictionary encoded, or it was using dictionary encoding but
- // a new data page has switched back to use plain encoding. For both cases we should
- // return plain vector.
- currentVector = cometVector;
- return currentVector;
- }
-
- // We should already re-initiate `CometDictionary` here because `Data.importVector` API will
- // release the previous dictionary vector and create a new one.
- Dictionary arrowDictionary = importer.getProvider().lookup(dictionaryEncoding.getId());
- CometPlainVector dictionaryVector =
- new CometPlainVector(arrowDictionary.getVector(), useDecimal128, isUuid);
Review Comment:
This is valid only for primitive types
##########
common/src/main/java/org/apache/comet/parquet/NativeBatchReader.java:
##########
@@ -266,7 +268,8 @@ public void init() throws URISyntaxException, IOException {
//// Create Column readers
List<ColumnDescriptor> columns = requestedSchema.getColumns();
- int numColumns = columns.size();
Review Comment:
This yields leaf fields which is not what we want.
##########
common/src/main/java/org/apache/comet/parquet/NativeBatchReader.java:
##########
@@ -469,19 +473,23 @@ private int loadNextBatch() throws Throwable {
importer = new CometSchemaImporter(ALLOCATOR);
List<ColumnDescriptor> columns = requestedSchema.getColumns();
- for (int i = 0; i < columns.size(); i++) {
Review Comment:
This also operates on leaf fields which, again, is not what we want.
--
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.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: handle loading of complex types into CometVector correctly in iceberg_compat scans [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on PR #1279:
URL: https://github.com/apache/datafusion-comet/pull/1279#issuecomment-2588930304
@andygrove @mbutrovich
--
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.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org