CurtHagenlocher commented on code in PR #41048:
URL: https://github.com/apache/arrow/pull/41048#discussion_r1554997906
##########
csharp/src/Apache.Arrow/Arrays/StringArray.cs:
##########
@@ -26,6 +26,8 @@ public class StringArray: BinaryArray, IReadOnlyList<string>
{
public static readonly Encoding DefaultEncoding = Encoding.UTF8;
+ private Dictionary<Encoding, List<string>> materializedStringStore =
new Dictionary<Encoding, List<string>>();
Review Comment:
Can the dictionary be allocated lazily so as not to pay the allocation cost
when the feature is not in use?
Can the materialized list be a `string[]` instead of a `List<string>` to
save an allocation?
##########
csharp/src/Apache.Arrow/Arrays/StringArray.cs:
##########
@@ -93,6 +100,30 @@ public string GetString(int index, Encoding encoding =
default)
}
}
+ public void Materialize(Encoding encoding = default)
Review Comment:
Please add doc comments indicating that these methods aren't
concurrency-safe with each other or with the value accessors. The codebase is
fairly inconsistent in its use of doc comments, but I believe the other array
types are all immutable and concurrency-safe and so this difference is worth
calling out.
--
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]
