CurtHagenlocher opened a new issue, #446:
URL: https://github.com/apache/arrow-dotnet/issues/446

   `ArrowArrayConcatenator.Concatenate` still cannot concatenate two array 
types after the fix for #443 (#444). Seen on `main` at 931db0e and in 
Apache.Arrow 23.0.0 (.NET 8, Windows 11).
   
   1. **Null arrays are refused.** `NullType` has no case in 
`ArrayDataConcatenationVisitor`, so it reaches the catch-all 
`Visit(IArrowType)` (`ArrayDataConcatenator.cs:553-556`). This happens at the 
top level and for a null-typed child, e.g. a struct field:
      ```
      NotImplementedException: Concatenation for null is not supported yet.
      ```
   
   2. **Dictionary arrays lose their dictionary.** `DictionaryType` derives 
from `FixedWidthType`, so a dictionary array goes through 
`Visit(FixedWidthType)` (`:118-126`). That concatenates the index buffers but 
builds the result without a `Dictionary`. This fails even when every input 
shares one dictionary:
      - through `ArrowArrayConcatenator`, when the result is built: 
`ArgumentException: Dictionary must not be null` (from `DictionaryArray..ctor`);
      - through `ArrayDataConcatenator` directly, **silently**: it returns a 
dictionary-typed `ArrayData` of the right length with `Dictionary == null`, 
which fails only when something later builds an array from it.
   
   **Expected:**
   - **Null:** a null array of the combined length. A null array has no 
buffers, so there is nothing to copy.
   - **Dictionary:** the result keeps a dictionary.
     - When the inputs share one dictionary, concatenating the indices and 
keeping that dictionary is enough.
     - When they differ, one always-correct option is to concatenate the 
dictionaries and shift each input's indices by the combined length of the 
dictionaries before it. Unifying the dictionaries would give a smaller result. 
Either way, the combined dictionary can outgrow the index type, which should be 
an error rather than a wraparound.
   
   **Repro** (console app referencing `main` at 931db0e):
   
   ```csharp
   using Apache.Arrow;
   using Apache.Arrow.Types;
   
   static void Run(string name, Func<string> body)
   {
       try { Console.WriteLine($"{name}: {body()}"); }
       catch (Exception ex) { Console.WriteLine($"{name}: {ex.GetType().Name}: 
{ex.Message}"); }
   }
   
   static DictionaryArray Dict(string[] dictionary, params int[] indices) => 
new(
       new DictionaryType(Int32Type.Default, StringType.Default, false),
       new Int32Array.Builder().AppendRange(indices).Build(),
       new StringArray.Builder().AppendRange(dictionary).Build());
   
   Run("1. null", () =>
       ArrowArrayConcatenator.Concatenate(new IArrowArray[] { new NullArray(3), 
new NullArray(2) }).Length.ToString());
   
   Run("1b. struct<null>", () =>
   {
       var type = new StructType(new[] { new Field("n", NullType.Default, true) 
});
       StructArray S(int n) => new(type, n, new IArrowArray[] { new 
NullArray(n) }, ArrowBuffer.Empty, 0);
       return ArrowArrayConcatenator.Concatenate(new IArrowArray[] { S(3), S(2) 
}).Length.ToString();
   });
   
   Run("2. dictionary, same dictionary", () =>
       ArrowArrayConcatenator.Concatenate(new IArrowArray[] { Dict(new[] { "a", 
"b" }, 0, 1), Dict(new[] { "a", "b" }, 1, 0) }).Length.ToString());
   
   Run("2b. dictionary, different dictionaries", () =>
       ArrowArrayConcatenator.Concatenate(new IArrowArray[] { Dict(new[] { "a", 
"b" }, 0, 1), Dict(new[] { "x", "y" }, 1, 0) }).Length.ToString());
   
   Run("2c. ArrayDataConcatenator, dictionary", () =>
   {
       var data = ArrayDataConcatenator.Concatenate(new[] { Dict(new[] { "a", 
"b" }, 0, 1).Data, Dict(new[] { "a", "b" }, 1, 0).Data });
       return $"type {data.DataType.Name}, length {data.Length}, dictionary 
{(data.Dictionary is null ? "null" : "present")}";
   });
   ```
   
   Output:
   
   ```
   1. null: NotImplementedException: Concatenation for null is not supported 
yet.
   1b. struct<null>: NotImplementedException: Concatenation for null is not 
supported yet.
   2. dictionary, same dictionary: ArgumentException: Dictionary must not be 
null
   2b. dictionary, different dictionaries: ArgumentException: Dictionary must 
not be null
   2c. ArrayDataConcatenator, dictionary: type dictionary, length 4, dictionary 
null
   ```
   
   Found while fixing #443. The null case is reachable from Parquet: pyarrow 
writes `pa.null()` columns with the Null logical type, and a reader that 
concatenates pages or row groups hits it.
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to