mr-smidge commented on a change in pull request #7654:
URL: https://github.com/apache/arrow/pull/7654#discussion_r458452083



##########
File path: csharp/src/Apache.Arrow/Arrays/Date64Array.cs
##########
@@ -15,56 +15,94 @@
 
 using Apache.Arrow.Types;
 using System;
-using System.Collections.Generic;
-using Apache.Arrow.Memory;
 
 namespace Apache.Arrow
 {
+    /// <summary>
+    /// The <see cref="Date64Array"/> class holds an array of dates in the 
<c>Date64</c> format, where each date is
+    /// stored as the number of milliseconds since the dawn of (UNIX) time, 
excluding leap seconds.
+    /// </summary>
     public class Date64Array: PrimitiveArray<long>
     {
+        private static readonly DateTime _epoch = new DateTime(1970, 1, 1, 0, 
0, 0, DateTimeKind.Unspecified);
+
         public Date64Array(
             ArrowBuffer valueBuffer, ArrowBuffer nullBitmapBuffer,
             int length, int nullCount, int offset)
             : this(new ArrayData(Date64Type.Default, length, nullCount, offset,
                 new[] { nullBitmapBuffer, valueBuffer }))
         { }
 
-        public class Builder : PrimitiveArrayBuilder<DateTimeOffset, long, 
Date64Array, Builder>
+        /// <summary>
+        /// The <see cref="Builder"/> class can be used to fluently build <see 
cref="Date64Array"/> objects.
+        /// </summary>
+        public class Builder : DateArrayBuilder<long, Date64Array, Builder>
         {
-            internal class DateBuilder: PrimitiveArrayBuilder<long, 
Date64Array, DateBuilder>
+            private class DateBuilder: PrimitiveArrayBuilder<long, 
Date64Array, DateBuilder>
             {
                 protected override Date64Array Build(
                     ArrowBuffer valueBuffer, ArrowBuffer nullBitmapBuffer,
                     int length, int nullCount, int offset) =>
                     new Date64Array(valueBuffer, nullBitmapBuffer, length, 
nullCount, offset);
-            } 
+            }
 
+            /// <summary>
+            /// Construct a new instance of the <see cref="Builder"/> class.
+            /// </summary>
             public Builder() : base(new DateBuilder()) { }
 
-            protected override long ConvertTo(DateTimeOffset value)
+            protected override long Convert(DateTime dateTime)
+            {
+                return (long)(dateTime - _epoch).TotalMilliseconds;
+            }
+
+            protected override long Convert(DateTimeOffset dateTimeOffset)
             {
-                return value.ToUnixTimeMilliseconds();
+                return (long)(dateTimeOffset.Date - _epoch).TotalMilliseconds;

Review comment:
       > ... at this very moment in time, the exact same number of days have 
passed since the Unix Epoch for me and for you, even though we are in different 
time zones and our local calendars may be on different days
   
   Ok, I agree with this, because the UNIX Epoch is a point in time rather than 
a date, and a "day" in this context must therefore be a block of 24 hours 
rather than a box on a calendar.
   
   Alternatively, if someone said "number of days passed since 1st Jan 1970", 
then a "day" must be a box on a calendar, because no time element has been 
mentioned: only dates.  This day count could indeed be different for us in 
different time zones at the very same moment in time.
   
   I'm happy with the idea that the `DateXX` array builders can accept a 
`DateTimeOffset` and interpret it as the number of 24-hour blocks since the 
UNIX Epoch.  I'll ensure this is fully documented, as I don't find this the 
intuitive interpretation and I'm sure I'm not alone here.
   
   Thank you for clarifying :smile:.




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

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


Reply via email to