mr-smidge commented on a change in pull request #7654:
URL: https://github.com/apache/arrow/pull/7654#discussion_r463886892
##########
File path: csharp/src/Apache.Arrow/Arrays/Date64Array.cs
##########
@@ -15,56 +15,103 @@
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, in multiples of
+ /// 86400000.
+ /// </summary>
public class Date64Array: PrimitiveArray<long>
{
+ private const long MillisecondsPerDay = 86400000;
+
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)
+ {
+ var dateTimeOffset = new DateTimeOffset(
+ DateTime.SpecifyKind(dateTime.Date,
DateTimeKind.Unspecified),
+ TimeSpan.Zero);
+ return dateTimeOffset.ToUnixTimeMilliseconds();
+ }
+
+ protected override long Convert(DateTimeOffset dateTimeOffset)
{
- return value.ToUnixTimeMilliseconds();
+ // The internal value stored for a DateTimeOffset can be
thought of as the number of milliseconds,
+ // in multiples of 86400000, that have passed since the UNIX
epoch. It is not the same as what would
+ // result from encoding the date from the DateTimeOffset.Date
property.
+ long millis = dateTimeOffset.ToUnixTimeMilliseconds();
+ long days = millis / MillisecondsPerDay;
+ return (millis < 0 ? days - 1 : days) * MillisecondsPerDay;
Review comment:
An alternative to this code might be:
```c#
// Similar to the code in Date32Array.
return (long)(dateTimeOffset.UtcDateTime.Date - _epochDate).TotalDays *
MillisecondsPerDay;
```
Or even:
```c#
// No maths, but requires making another DateTimeOffset only to throw it
away again.
return new DateTimeOffset(dateTimeOffset.UtcDateTime.Date,
TimeSpan.Zero).ToUnixMilliseconds();
```
@eerhardt , please let me know if you have a preference between any of these.
##########
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:
I've now updated the PR so that treatment of `DateTimeOffset` is as
discussed above: it is converted to UTC first and then the days are counted, so
that the input is treated as a point in time rather than a calendar date/time.
The docstrings all explain this behaviour (hopefully clearly.
----------------------------------------------------------------
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
