You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@gluten.apache.org by "lgbo-ustc (via GitHub)" <gi...@apache.org> on 2025/05/27 03:03:27 UTC

[PR] [GLUTEN-9752][Flink] Support array/map in row/vector conversion [incubator-gluten]

lgbo-ustc opened a new pull request, #9757:
URL: https://github.com/apache/incubator-gluten/pull/9757

   ## What changes were proposed in this pull request?
   
   (Please fill in changes proposed in this fix)
   
   Fixes: #9752 , #9754
   
   ## How was this patch tested?
   
   (Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
   unit tests
   
   (If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
   
   


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9752][Flink] Support array/map in row/vector conversion [incubator-gluten]

Posted by "CodiumAI-Agent (via GitHub)" <gi...@apache.org>.

CodiumAI-Agent commented on PR #9757:
URL: https://github.com/apache/incubator-gluten/pull/9757#issuecomment-2911012432

   **[Persistent review](https://github.com/apache/incubator-gluten/pull/9757#issuecomment-2910971268)** updated to latest commit https://github.com/apache/incubator-gluten/commit/d2df42653460c309271fba95ea5216be4b722018


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9752][FLINK] Support array/map in row/vector conversion [incubator-gluten]

Posted by "lgbo-ustc (via GitHub)" <gi...@apache.org>.

lgbo-ustc merged PR #9757:
URL: https://github.com/apache/incubator-gluten/pull/9757


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9752][Flink] Support array/map in row/vector conversion [incubator-gluten]

Posted by "lgbo-ustc (via GitHub)" <gi...@apache.org>.

lgbo-ustc commented on PR #9757:
URL: https://github.com/apache/incubator-gluten/pull/9757#issuecomment-2918550097

   Hi @PHILO-HE , Could you take a look? I will merge it if there are no further comments


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9752][Flink] Support array/map in row/vector conversion [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #9757:
URL: https://github.com/apache/incubator-gluten/pull/9757#issuecomment-2910968039

   https://github.com/apache/incubator-gluten/issues/9752


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9752][Flink] Support array/map in row/vector conversion [incubator-gluten]

Posted by "lgbo-ustc (via GitHub)" <gi...@apache.org>.

lgbo-ustc commented on PR #9757:
URL: https://github.com/apache/incubator-gluten/pull/9757#issuecomment-2911011108

   @CodiumAI-Agent /review


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9752][Flink] Support array/map in row/vector conversion [incubator-gluten]

Posted by "CodiumAI-Agent (via GitHub)" <gi...@apache.org>.

CodiumAI-Agent commented on PR #9757:
URL: https://github.com/apache/incubator-gluten/pull/9757#issuecomment-2914708534

   **[Persistent review](https://github.com/apache/incubator-gluten/pull/9757#issuecomment-2910971268)** updated to latest commit https://github.com/apache/incubator-gluten/commit/dae25f12ddc07dc02361cbc5034cee1d8dccc8b9


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9752][Flink] Support array/map in row/vector conversion [incubator-gluten]

Posted by "CodiumAI-Agent (via GitHub)" <gi...@apache.org>.

CodiumAI-Agent commented on PR #9757:
URL: https://github.com/apache/incubator-gluten/pull/9757#issuecomment-2910971268

   ## PR Reviewer Guide 🔍
   
   Here are some key observations to aid the review process:
   
   <table>
   <tr><td>
   
   **🎫 Ticket compliance analysis ✅**
   
   
   
   **[9752](https://github.com/apache/incubator-gluten/issues/9752) - PR Code Verified**
   
   Compliant requirements:
   
   - Support ArrayType conversion in LogicalTypeConverter.
   - Implement ListVectorAccessor for array vectors.
   - Implement ArrayVectorWriter.
   - Add unit tests covering array types in ScanTest.
   
   Requires further human verification:
   
   - Handling of null or empty arrays at runtime.
   
   
   
   
   
   **[9754](https://github.com/apache/incubator-gluten/issues/9754) - PR Code Verified**
   
   Compliant requirements:
   
   - Support MapType conversion in LogicalTypeConverter.
   - Implement MapVectorAccessor.
   - Implement MapVectorWriter.
   - Add unit tests covering map types in ScanTest.
   
   Requires further human verification:
   
   - Handling of null or empty maps at runtime.
   
   
   
   </td></tr>
   <tr><td>⏱️&nbsp;<strong>Estimated effort to review</strong>: 4 🔵🔵🔵🔵⚪</td></tr>
   <tr><td>🧪&nbsp;<strong>PR contains tests</strong></td></tr>
   <tr><td>🔒&nbsp;<strong>No security concerns identified</strong></td></tr>
   <tr><td>⚡&nbsp;<strong>Recommended focus areas for review</strong><br><br>
   
   <details><summary><a href='https://github.com/apache/incubator-gluten/pull/9757/files#diff-bcb790d33c3348eea0bd7acd49e53189c7b581defa2967157de02e10cd176263R158-R207'><strong>Null handling</strong></a>
   
   ListVectorAccessor and MapVectorAccessor lack null or empty checks for vectors, which may lead to errors when encountering null entries.
   </summary>
   
   ```java
   class ListVectorAccessor extends ArrowVectorAccessor {
       private ListVector vector;
       private ArrowVectorAccessor elementAccessor;
   
       public ListVectorAccessor(FieldVector vector) {
           this.vector = (ListVector) vector;
           FieldVector elementVector = this.vector.getDataVector();
           this.elementAccessor = ArrowVectorAccessor.create(elementVector);
       }
   
       @Override
       public Object get(int rowIndex) {
           int startIndex = vector.getElementStartIndex(rowIndex);
           int endIndex = vector.getElementEndIndex(rowIndex);
           Object[] elements = new Object[endIndex - startIndex];
           for (int i = startIndex; i < endIndex; i++) {
               elements[i - startIndex] = elementAccessor.get(i);
           }
           return new GenericArrayData(elements);
       }
   }
   
   // In Arrow, the internal implementation of a map vector is an array vector.
   class MapVectorAccessor extends ArrowVectorAccessor {
       private final MapVector vector;
       private StructVector entriesVector;
       private ArrowVectorAccessor keyAccessor;
       private ArrowVectorAccessor valueAccessor;
       public MapVectorAccessor(FieldVector vector) {
           this.vector = (MapVector) vector;
           this.entriesVector = (StructVector) this.vector.getDataVector();
           FieldVector keyVector = this.entriesVector.getChild(MapVector.KEY_NAME);
           FieldVector valueVector = this.entriesVector.getChild(MapVector.VALUE_NAME);
           this.keyAccessor = ArrowVectorAccessor.create(keyVector);
           this.valueAccessor = ArrowVectorAccessor.create(valueVector);
       }
   
       @Override
       public Object get(int rowIndex) {
           int startIndex = vector.getElementStartIndex(rowIndex);
           int endIndex = vector.getElementEndIndex(rowIndex);
           Map<Object, Object> mapEntries = new HashMap<>();
           for (int i = startIndex; i < endIndex; i++) {
               Object key = keyAccessor.get(i);
               Object value = valueAccessor.get(i);
               mapEntries.put(key, value);
           }
           return new GenericMapData(mapEntries);
       }
   }
   ```
   
   </details>
   
   <details><summary><a href='https://github.com/apache/incubator-gluten/pull/9757/files#diff-bcb790d33c3348eea0bd7acd49e53189c7b581defa2967157de02e10cd176263R181-R207'><strong>Map ordering</strong></a>
   
   MapVectorAccessor uses HashMap, resulting in non-deterministic key ordering in output. Consider using LinkedHashMap for predictable ordering.
   </summary>
   
   ```java
   class MapVectorAccessor extends ArrowVectorAccessor {
       private final MapVector vector;
       private StructVector entriesVector;
       private ArrowVectorAccessor keyAccessor;
       private ArrowVectorAccessor valueAccessor;
       public MapVectorAccessor(FieldVector vector) {
           this.vector = (MapVector) vector;
           this.entriesVector = (StructVector) this.vector.getDataVector();
           FieldVector keyVector = this.entriesVector.getChild(MapVector.KEY_NAME);
           FieldVector valueVector = this.entriesVector.getChild(MapVector.VALUE_NAME);
           this.keyAccessor = ArrowVectorAccessor.create(keyVector);
           this.valueAccessor = ArrowVectorAccessor.create(valueVector);
       }
   
       @Override
       public Object get(int rowIndex) {
           int startIndex = vector.getElementStartIndex(rowIndex);
           int endIndex = vector.getElementEndIndex(rowIndex);
           Map<Object, Object> mapEntries = new HashMap<>();
           for (int i = startIndex; i < endIndex; i++) {
               Object key = keyAccessor.get(i);
               Object value = valueAccessor.get(i);
               mapEntries.put(key, value);
           }
           return new GenericMapData(mapEntries);
       }
   }
   ```
   
   </details>
   
   <details><summary><a href='https://github.com/apache/incubator-gluten/pull/9757/files#diff-afa8cbf7bf66b8228476179fcf77ecd306a21e95903b7cb3c8c6e6139835b714R47-R82'><strong>Type matching</strong></a>
   
   Converters map uses exact class key matching, which won’t match subclasses or parameterized logical types. Consider instanceof checks or a more flexible registry.
   </summary>
   
   ```java
   private static Map<Class<?>, VLTypeConverter> converters = Map.ofEntries(
       Map.entry(BooleanType.class, logicalType -> new io.github.zhztheplayer.velox4j.type.BooleanType()),
       Map.entry(IntType.class, logicalType -> new io.github.zhztheplayer.velox4j.type.IntegerType()),
       Map.entry(BigIntType.class, logicalType -> new io.github.zhztheplayer.velox4j.type.BigIntType()),
       Map.entry(DoubleType.class, logicalType -> new io.github.zhztheplayer.velox4j.type.DoubleType()),
       Map.entry(VarCharType.class, logicalType -> new io.github.zhztheplayer.velox4j.type.VarCharType()),
       //  TODO: may need precision
       Map.entry(TimestampType.class, logicalType -> new io.github.zhztheplayer.velox4j.type.TimestampType()),
       Map.entry(DecimalType.class, logicalType -> {
           DecimalType decimalType = (DecimalType) logicalType;
           return new io.github.zhztheplayer.velox4j.type.DecimalType(
               decimalType.getPrecision(),
               decimalType.getScale());
       }),
       Map.entry(DayTimeIntervalType.class, logicalType -> new io.github.zhztheplayer.velox4j.type.BigIntType()),
       Map.entry(RowType.class, logicalType -> {
           RowType flinkRowType = (RowType) logicalType;
           List<Type> fieldTypes = flinkRowType.getChildren().stream().
               map(LogicalTypeConverter::toVLType).
               collect(Collectors.toList());
           return new io.github.zhztheplayer.velox4j.type.RowType(
               flinkRowType.getFieldNames(),
               fieldTypes);
       }),
       Map.entry(ArrayType.class, logicalType -> {
           ArrayType arrayType = (ArrayType) logicalType;
           Type elementType = toVLType(arrayType.getElementType());
           return io.github.zhztheplayer.velox4j.type.ArrayType.create(elementType);
       }),
       Map.entry(MapType.class, logicalType -> {
           MapType mapType = (MapType) logicalType;
           Type keyType = toVLType(mapType.getKeyType());
           Type valueType = toVLType(mapType.getValueType());
           return io.github.zhztheplayer.velox4j.type.MapType.create(keyType, valueType);
       })
   );
   ```
   
   </details>
   
   </td></tr>
   </table>
   


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9752][Flink] Support array/map in row/vector conversion [incubator-gluten]

Posted by "lgbo-ustc (via GitHub)" <gi...@apache.org>.

lgbo-ustc commented on PR #9757:
URL: https://github.com/apache/incubator-gluten/pull/9757#issuecomment-2914707156

   @KevinyhZou 


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org

Re: [PR] [GLUTEN-9752][Flink] Support array/map in row/vector conversion [incubator-gluten]

Posted by "lgbo-ustc (via GitHub)" <gi...@apache.org>.

lgbo-ustc commented on PR #9757:
URL: https://github.com/apache/incubator-gluten/pull/9757#issuecomment-2910969989

   @CodiumAI-Agent /review


-- 
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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org