CurtHagenlocher commented on code in PR #41539:
URL: https://github.com/apache/arrow/pull/41539#discussion_r1596200190


##########
csharp/src/Apache.Arrow/Arrays/BinaryArray.cs:
##########
@@ -380,5 +380,40 @@ public ReadOnlySpan<byte> GetBytes(int index, out bool 
isNull)
         }
 
         IEnumerator IEnumerable.GetEnumerator() => 
((IEnumerable<byte[]>)this).GetEnumerator();
+
+        int ICollection<byte[]>.Count => Length;
+        bool ICollection<byte[]>.IsReadOnly => true;
+        void ICollection<byte[]>.Add(byte[]? item) => throw new 
NotSupportedException("Collection is read-only.");
+        bool ICollection<byte[]>.Remove(byte[]? item) => throw new 
NotSupportedException("Collection is read-only.");
+        void ICollection<byte[]>.Clear() => throw new 
NotSupportedException("Collection is read-only.");
+
+        bool ICollection<byte[]>.Contains(byte[] item)
+        {
+            for (int index = 0; index < Length; index++)
+            {
+                var foo = GetBytes(index);
+                if(IsSpanDataEqual(foo, item))
+                //if (foo == item)

Review Comment:
   remove?



##########
csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs:
##########
@@ -86,5 +86,30 @@ IEnumerator IEnumerable.GetEnumerator()
                 yield return IsValid(index) ? Values[index] : null;
             }
         }
+
+        int ICollection<T?>.Count => Length;
+        bool ICollection<T?>.IsReadOnly => true;
+        void ICollection<T?>.Add(T? item) => throw new 
NotSupportedException("Collection is read-only.");
+        bool ICollection<T?>.Remove(T? item) => throw new 
NotSupportedException("Collection is read-only.");
+        void ICollection<T?>.Clear() => throw new 
NotSupportedException("Collection is read-only.");
+
+        bool ICollection<T?>.Contains(T? item)

Review Comment:
   This could be probably be optimized as something like
   ```
               if (value == null)
               {
                   return NullCount > 0;
               }
               else
               {
                   ReadOnlySpan<T> values = Values;
                   while (values.Length > 0)
                   {
                       int index = Values.IndexOf(value.Value);
                       if (index < 0 || IsValid(index)) { return index >= 0; }
                       values = values.Slice(index + 1);
                   }
                   return false;
               }
   ```



##########
csharp/src/Apache.Arrow/Arrays/BinaryArray.cs:
##########
@@ -380,5 +380,40 @@ public ReadOnlySpan<byte> GetBytes(int index, out bool 
isNull)
         }
 
         IEnumerator IEnumerable.GetEnumerator() => 
((IEnumerable<byte[]>)this).GetEnumerator();
+
+        int ICollection<byte[]>.Count => Length;
+        bool ICollection<byte[]>.IsReadOnly => true;
+        void ICollection<byte[]>.Add(byte[]? item) => throw new 
NotSupportedException("Collection is read-only.");
+        bool ICollection<byte[]>.Remove(byte[]? item) => throw new 
NotSupportedException("Collection is read-only.");
+        void ICollection<byte[]>.Clear() => throw new 
NotSupportedException("Collection is read-only.");
+
+        bool ICollection<byte[]>.Contains(byte[] item)
+        {
+            for (int index = 0; index < Length; index++)
+            {
+                var foo = GetBytes(index);
+                if(IsSpanDataEqual(foo, item))
+                //if (foo == item)
+                    return true;
+            }
+
+            return false;
+        }
+
+        private static bool IsSpanDataEqual(ReadOnlySpan<byte> span1, 
ReadOnlySpan<byte> span2)
+        {
+            if (span1.Length != span2.Length)

Review Comment:
   Looking at the implementation of SequenceEqual, this extra comparison 
doens't seem to help.



##########
csharp/src/Apache.Arrow/Arrays/PrimitiveArray.cs:
##########
@@ -86,5 +86,30 @@ IEnumerator IEnumerable.GetEnumerator()
                 yield return IsValid(index) ? Values[index] : null;
             }
         }
+
+        int ICollection<T?>.Count => Length;
+        bool ICollection<T?>.IsReadOnly => true;
+        void ICollection<T?>.Add(T? item) => throw new 
NotSupportedException("Collection is read-only.");
+        bool ICollection<T?>.Remove(T? item) => throw new 
NotSupportedException("Collection is read-only.");
+        void ICollection<T?>.Clear() => throw new 
NotSupportedException("Collection is read-only.");
+
+        bool ICollection<T?>.Contains(T? item)

Review Comment:
   (The same applies to the other Index methods.)



##########
csharp/src/Apache.Arrow/Arrays/StringArray.cs:
##########
@@ -164,5 +164,30 @@ IEnumerator<string> IEnumerable<string>.GetEnumerator()
         }
 
         IEnumerator IEnumerable.GetEnumerator() => 
((IEnumerable<string>)this).GetEnumerator();
+
+        int ICollection<string>.Count => Length;
+        bool ICollection<string>.IsReadOnly => true;
+        void ICollection<string>.Add(string item) => throw new 
NotSupportedException("Collection is read-only.");
+        bool ICollection<string>.Remove(string item) => throw new 
NotSupportedException("Collection is read-only.");
+        void ICollection<string>.Clear() => throw new 
NotSupportedException("Collection is read-only.");
+
+        bool ICollection<string>.Contains(string item)

Review Comment:
   This could be optimized by using a similar null vs not-null fork and by 
using the materializedStringStore (if present) or converting the search string 
to binary and doing a binary comparison if the string list hasn't been 
materialized.



-- 
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