[GitHub] [arrow] DanTm99 commented on a diff in pull request #34125: GH-34076: [C#] Allow schema fields with duplicate names

["DanTm99 (via GitHub)" <gi...@apache.org>]

DanTm99 commented on code in PR #34125:
URL: https://github.com/apache/arrow/pull/34125#discussion_r1103597114


##########
csharp/src/Apache.Arrow/Schema.cs:
##########
@@ -22,32 +22,31 @@ namespace Apache.Arrow
 {
     public partial class Schema
     {
-        public IReadOnlyDictionary<string, Field> Fields
-        {
-            get => _fieldsDictionary;
-        }
+        [Obsolete("Use `FieldList` instead")]
+        public IReadOnlyDictionary<string, Field> Fields => _fieldsDictionary;

Review Comment:
   When enumerating the key value pairs, either:
   - multiple key value pairs would have the same key
   - some fields would be missing
   
   In the remarks for the `IReadOnlyDictionary` docs, it is stated that:
   > Each pair must have a unique key.
   
   This means the former would violate this and could lead to behaviour that the user would not expect.
   
   If both 
   - a single field is returned (instead of throwing an exception) when someone tries accessing a field by name for which there are multiple fields
   - duplicate fields are skipped when enumerating
   
   then this is equivalent to just skipping duplicate keys when building the dictionary.
   
   This API can still be supported by skipping duplicate keys when building the dictionary, but that would lead to the dictionary being an inaccurate representation of the state of the schema when there are multiple fields with the same name. Because of this, deprecating it could make it clear that using it is to be avoided.



-- 
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@arrow.apache.org

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