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:
IF we were to use a custom `IReadOnlyDictionary`. when enumerating the key
value pairs either:
- multiple key value pairs would have the same key
- fields with duplicate keys would be missing
In the remarks for the `IReadOnlyDictionary` Microsoft 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 an exception being thrown) 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
