[
https://issues.apache.org/jira/browse/AVRO-3478?focusedWorklogId=782641&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-782641
]
ASF GitHub Bot logged work on AVRO-3478:
----------------------------------------
Author: ASF GitHub Bot
Created on: 18/Jun/22 19:02
Start Date: 18/Jun/22 19:02
Worklog Time Spent: 10m
Work Description: KalleOlaviNiemitalo commented on code in PR #1630:
URL: https://github.com/apache/avro/pull/1630#discussion_r901004450
##########
lang/csharp/src/apache/main/AvroDuration.cs:
##########
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+using System;
+
+namespace Avro
+{
+ /// <summary>
+ /// Represents a duration.
+ /// </summary>
+ public struct AvroDuration : IComparable, IComparable<AvroDuration>,
IEquatable<AvroDuration>
+ {
+ /// <summary>
+ /// Initializes a new instance of the <see cref="AvroDuration" />
struct.
+ /// </summary>
+ /// <param name="months">Number of months.</param>
+ /// <param name="days">Number of days.</param>
+ /// <param name="milliseconds">Number of milliseconds.</param>
+ public AvroDuration(int months, int days, int milliseconds)
+ {
+ Months = months;
+ Days = days;
+ Milliseconds = milliseconds;
+ }
+
+ /// <summary>
+ /// Gets/sets the number of months represented by the current <see
cref="AvroDuration" />.
+ /// </summary>
+ /// <value>
+ /// Number of months.
+ /// </value>
+ public int Months { get; set; }
+
+ /// <summary>
+ /// Gets/sets the number of days represented by the current <see
cref="AvroDuration" />.
+ /// </summary>
+ /// <value>
+ /// Number of days.
+ /// </value>
+ public int Days { get; set; }
+
+ /// <summary>
+ /// Gets/sets the number of milliseconds represented by the current
<see cref="AvroDuration" />.
+ /// </summary>
+ /// <value>
+ /// Number of milliseconds.
+ /// </value>
+ public int Milliseconds { get; set; }
+
+ /// <summary>
+ /// Converts the current <see cref="AvroDuration" /> to a string.
+ /// </summary>
+ /// <returns>
+ /// A string representation of the duration value.
+ /// </returns>
+ public override string ToString()
+ {
+ return $"(M{Months},D{Days},MS{Milliseconds})";
+ }
+
+ /// <summary>
+ /// Implements the operator ==.
+ /// </summary>
+ /// <param name="left">The left.</param>
+ /// <param name="right">The right.</param>
+ /// <returns>
+ /// The result of the operator.
+ /// </returns>
+ public static bool operator ==(AvroDuration left, AvroDuration right)
+ {
+ return left.Equals(right);
+ }
+
+ /// <summary>
+ /// Implements the operator !=.
+ /// </summary>
+ /// <param name="left">The left.</param>
+ /// <param name="right">The right.</param>
+ /// <returns>
+ /// The result of the operator.
+ /// </returns>
+ public static bool operator !=(AvroDuration left, AvroDuration right)
+ {
+ return !left.Equals(right);
+ }
+
+ /// <summary>
+ /// Implements the operator >.
+ /// </summary>
+ /// <param name="left">The left.</param>
+ /// <param name="right">The right.</param>
+ /// <returns>
+ /// The result of the operator.
+ /// </returns>
+ public static bool operator >(AvroDuration left, AvroDuration right)
+ {
+ return left.CompareTo(right) > 0;
+ }
+
+ /// <summary>
+ /// Implements the operator >=.
+ /// </summary>
+ /// <param name="left">The left.</param>
+ /// <param name="right">The right.</param>
+ /// <returns>
+ /// The result of the operator.
+ /// </returns>
+ public static bool operator >=(AvroDuration left, AvroDuration right)
+ {
+ return left.CompareTo(right) >= 0;
+ }
+
+ /// <summary>
+ /// Implements the operator <.
+ /// </summary>
+ /// <param name="left">The left.</param>
+ /// <param name="right">The right.</param>
+ /// <returns>
+ /// The result of the operator.
+ /// </returns>
+ public static bool operator <(AvroDuration left, AvroDuration right)
+ {
+ return left.CompareTo(right) < 0;
+ }
+
+ /// <summary>
+ /// Implements the operator <=.
+ /// </summary>
+ /// <param name="left">The left.</param>
+ /// <param name="right">The right.</param>
+ /// <returns>
+ /// The result of the operator.
+ /// </returns>
+ public static bool operator <=(AvroDuration left, AvroDuration right)
+ {
+ return left.CompareTo(right) <= 0;
+ }
+
+ /// <summary>
+ /// Returns a value that indicates whether the current <see
cref="AvroDuration" /> and a specified object
+ /// have the same value.
+ /// </summary>
+ /// <param name="obj">The object to compare.</param>
+ /// <returns>
+ /// true if the obj argument is an <see cref="AvroDuration" /> object,
and its value
+ /// is equal to the value of the current <see cref="AvroDuration" />
instance; otherwise false.
+ /// </returns>
+ public override bool Equals(object obj)
+ {
+ return (obj is AvroDuration duration) && Equals(duration);
+ }
+
+ /// <summary>
+ /// Returns the hash code for the current <see cref="AvroDuration" />.
+ /// </summary>
+ /// <returns>
+ /// The hash code.
+ /// </returns>
+ public override int GetHashCode()
+ {
+ return Months ^ Days ^ Milliseconds;
+ }
+
+ /// <summary>
+ /// Compares the value of the current <see cref="AvroDuration" /> to
the value of another object.
+ /// </summary>
+ /// <param name="obj">The object to compare.</param>
+ /// <returns>
+ /// A value that indicates the relative order of the objects being
compared.
+ /// </returns>
+ public int CompareTo(object obj)
+ {
+ if (obj == null)
+ {
+ return 1;
+ }
+
+ if (obj is AvroDuration duration)
+ {
+ return CompareTo(duration);
+ }
+
+ return 1;
Review Comment:
If `obj` is neither `null` nor an `AvroDuration`, CompareTo(object obj)
should throw ArgumentException. That would be consistent with the
IComparable.CompareTo(object) documentation and with other implementations:
e.g. in AvroDecimal, System.Int32, and System.String.
##########
lang/csharp/src/apache/main/AvroDuration.cs:
##########
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+using System;
+
+namespace Avro
+{
+ /// <summary>
+ /// Represents a duration.
+ /// </summary>
+ public struct AvroDuration : IComparable, IComparable<AvroDuration>,
IEquatable<AvroDuration>
+ {
+ /// <summary>
+ /// Initializes a new instance of the <see cref="AvroDuration" />
struct.
+ /// </summary>
+ /// <param name="months">Number of months.</param>
+ /// <param name="days">Number of days.</param>
+ /// <param name="milliseconds">Number of milliseconds.</param>
+ public AvroDuration(int months, int days, int milliseconds)
+ {
+ Months = months;
+ Days = days;
+ Milliseconds = milliseconds;
+ }
+
+ /// <summary>
+ /// Gets/sets the number of months represented by the current <see
cref="AvroDuration" />.
+ /// </summary>
+ /// <value>
+ /// Number of months.
+ /// </value>
+ public int Months { get; set; }
Review Comment:
I would mildly prefer AvroDuration as an immutable type, with read-only
properties.
##########
lang/csharp/src/apache/main/AvroDuration.cs:
##########
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+using System;
+
+namespace Avro
+{
+ /// <summary>
+ /// Represents a duration.
+ /// </summary>
+ public struct AvroDuration : IComparable, IComparable<AvroDuration>,
IEquatable<AvroDuration>
+ {
+ /// <summary>
+ /// Initializes a new instance of the <see cref="AvroDuration" />
struct.
+ /// </summary>
+ /// <param name="months">Number of months.</param>
+ /// <param name="days">Number of days.</param>
+ /// <param name="milliseconds">Number of milliseconds.</param>
+ public AvroDuration(int months, int days, int milliseconds)
+ {
+ Months = months;
+ Days = days;
+ Milliseconds = milliseconds;
+ }
+
+ /// <summary>
+ /// Gets/sets the number of months represented by the current <see
cref="AvroDuration" />.
+ /// </summary>
+ /// <value>
+ /// Number of months.
+ /// </value>
+ public int Months { get; set; }
+
+ /// <summary>
+ /// Gets/sets the number of days represented by the current <see
cref="AvroDuration" />.
+ /// </summary>
+ /// <value>
+ /// Number of days.
+ /// </value>
+ public int Days { get; set; }
+
+ /// <summary>
+ /// Gets/sets the number of milliseconds represented by the current
<see cref="AvroDuration" />.
+ /// </summary>
+ /// <value>
+ /// Number of milliseconds.
+ /// </value>
+ public int Milliseconds { get; set; }
+
+ /// <summary>
+ /// Converts the current <see cref="AvroDuration" /> to a string.
+ /// </summary>
+ /// <returns>
+ /// A string representation of the duration value.
+ /// </returns>
+ public override string ToString()
+ {
+ return $"(M{Months},D{Days},MS{Milliseconds})";
+ }
+
+ /// <summary>
+ /// Implements the operator ==.
+ /// </summary>
+ /// <param name="left">The left.</param>
+ /// <param name="right">The right.</param>
+ /// <returns>
+ /// The result of the operator.
+ /// </returns>
+ public static bool operator ==(AvroDuration left, AvroDuration right)
+ {
+ return left.Equals(right);
+ }
+
+ /// <summary>
+ /// Implements the operator !=.
+ /// </summary>
+ /// <param name="left">The left.</param>
+ /// <param name="right">The right.</param>
+ /// <returns>
+ /// The result of the operator.
+ /// </returns>
+ public static bool operator !=(AvroDuration left, AvroDuration right)
+ {
+ return !left.Equals(right);
+ }
+
+ /// <summary>
+ /// Implements the operator >.
+ /// </summary>
+ /// <param name="left">The left.</param>
+ /// <param name="right">The right.</param>
+ /// <returns>
+ /// The result of the operator.
+ /// </returns>
+ public static bool operator >(AvroDuration left, AvroDuration right)
+ {
+ return left.CompareTo(right) > 0;
+ }
+
+ /// <summary>
+ /// Implements the operator >=.
+ /// </summary>
+ /// <param name="left">The left.</param>
+ /// <param name="right">The right.</param>
+ /// <returns>
+ /// The result of the operator.
+ /// </returns>
+ public static bool operator >=(AvroDuration left, AvroDuration right)
+ {
+ return left.CompareTo(right) >= 0;
+ }
+
+ /// <summary>
+ /// Implements the operator <.
+ /// </summary>
+ /// <param name="left">The left.</param>
+ /// <param name="right">The right.</param>
+ /// <returns>
+ /// The result of the operator.
+ /// </returns>
+ public static bool operator <(AvroDuration left, AvroDuration right)
+ {
+ return left.CompareTo(right) < 0;
+ }
+
+ /// <summary>
+ /// Implements the operator <=.
+ /// </summary>
+ /// <param name="left">The left.</param>
+ /// <param name="right">The right.</param>
+ /// <returns>
+ /// The result of the operator.
+ /// </returns>
+ public static bool operator <=(AvroDuration left, AvroDuration right)
+ {
+ return left.CompareTo(right) <= 0;
+ }
+
+ /// <summary>
+ /// Returns a value that indicates whether the current <see
cref="AvroDuration" /> and a specified object
+ /// have the same value.
+ /// </summary>
+ /// <param name="obj">The object to compare.</param>
+ /// <returns>
+ /// true if the obj argument is an <see cref="AvroDuration" /> object,
and its value
+ /// is equal to the value of the current <see cref="AvroDuration" />
instance; otherwise false.
+ /// </returns>
+ public override bool Equals(object obj)
+ {
+ return (obj is AvroDuration duration) && Equals(duration);
+ }
+
+ /// <summary>
+ /// Returns the hash code for the current <see cref="AvroDuration" />.
+ /// </summary>
+ /// <returns>
+ /// The hash code.
+ /// </returns>
+ public override int GetHashCode()
+ {
+ return Months ^ Days ^ Milliseconds;
+ }
+
+ /// <summary>
+ /// Compares the value of the current <see cref="AvroDuration" /> to
the value of another object.
+ /// </summary>
+ /// <param name="obj">The object to compare.</param>
+ /// <returns>
+ /// A value that indicates the relative order of the objects being
compared.
+ /// </returns>
+ public int CompareTo(object obj)
+ {
+ if (obj == null)
+ {
+ return 1;
+ }
+
+ if (obj is AvroDuration duration)
+ {
+ return CompareTo(duration);
+ }
+
+ return 1;
+ }
+
+ /// <summary>
+ /// Compares the value of the current <see cref="AvroDuration" /> to
the value of another
+ /// <see cref="AvroDuration" />.
+ /// </summary>
+ /// <param name="other">The <see cref="AvroDuration" /> to
compare.</param>
+ /// <returns>
+ /// A value that indicates the relative order of the <see
cref="AvroDuration" />
+ /// instances being compared.
+ /// </returns>
+ public int CompareTo(AvroDuration other)
+ {
+ return TotalMilliseconds.CompareTo(other.TotalMilliseconds);
+ }
+
+ // This is not a perfect total milliseconds value, because it assumes
that every month has 30 days.
+ private long TotalMilliseconds => (Months * 30L + Days) * 24L *
3600000L + Milliseconds;
Review Comment:
Perhaps it would be better to just delete the comparison interfaces and
operators (other than equality comparisons). That way, AvroDuration would not
need to assume a fixed number of days per month; the application can implement
its own `IComparer<AvroDuration>` if needed. The `System.DateSpan` proposal
<https://github.com/dotnet/runtime/issues/54400> has related discussion.
##########
lang/csharp/src/apache/main/Util/Duration.cs:
##########
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+using System;
+
+namespace Avro.Util
+{
+ /// <summary>
+ /// The 'duration' logical type.
+ /// </summary>
+ public class Duration : LogicalType<AvroDuration>
+ {
+ /// <summary>
+ /// The logical type name for Duration.
+ /// </summary>
+ public static readonly string LogicalTypeName = "duration";
+
+ /// <summary>
+ /// Initializes a new Duration logical type.
+ /// </summary>
+ public Duration()
+ : base(LogicalTypeName)
+ {
+ }
+
+ /// <inheritdoc/>
+ public override void ValidateSchema(LogicalSchema schema)
+ {
+ if (Schema.Type.Fixed != schema.BaseSchema.Tag)
+ {
+ throw new AvroTypeException($"'{LogicalTypeName}' can only be
used with an underlying bytes or fixed type");
+ }
+
+ if (((FixedSchema)schema.BaseSchema).Size != 12)
+ {
+ throw new AvroTypeException($"'{LogicalTypeName}' requires a
'size' property that is set to 12");
+ }
+ }
+
+ /// <inheritdoc/>
+ public override object ConvertToBaseValue(object logicalValue,
LogicalSchema schema)
+ {
+ AvroDuration duration = (AvroDuration)logicalValue;
+
+ byte[] baseValue = new byte[12];
+
+ BitConverter.GetBytes(duration.Months).CopyTo(baseValue, 0);
+ BitConverter.GetBytes(duration.Days).CopyTo(baseValue, 4);
+ BitConverter.GetBytes(duration.Milliseconds).CopyTo(baseValue, 8);
+
+ if (!BitConverter.IsLittleEndian)
+ {
+ Array.Reverse(baseValue, 0, 4);
+ Array.Reverse(baseValue, 4, 4);
+ Array.Reverse(baseValue, 8, 4);
+ }
Review Comment:
On netstandard2.1, this could use
[BinaryPrimitives.WriteInt32BigEndian](https://docs.microsoft.com/dotnet/api/system.buffers.binary.binaryprimitives.writeint32bigendian?view=netstandard-2.1)
for fewer memory allocations. That change is not urgent though.
##########
lang/csharp/src/apache/main/Util/LogicalType.cs:
##########
@@ -74,4 +75,32 @@ public virtual void ValidateSchema(LogicalSchema schema)
/// <param name="logicalValue">The logical value to test.</param>
public abstract bool IsInstanceOfLogicalType(object logicalValue);
}
+
+ /// <summary>
+ /// Base for all logical type implementations that are based on a struct
type.
+ /// </summary>
+ public abstract class LogicalType<T> : LogicalType
+ where T : struct
Review Comment:
That seems a good idea.
Issue Time Tracking
-------------------
Worklog Id: (was: 782641)
Remaining Estimate: 22h (was: 22h 10m)
Time Spent: 2h (was: 1h 50m)
> Support duration logical type in C#
> -----------------------------------
>
> Key: AVRO-3478
> URL: https://issues.apache.org/jira/browse/AVRO-3478
> Project: Apache Avro
> Issue Type: Improvement
> Components: csharp
> Reporter: Zoltan Csizmadia
> Priority: Minor
> Labels: pull-request-available
> Original Estimate: 24h
> Time Spent: 2h
> Remaining Estimate: 22h
>
> Support for "duration" logical type
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
