[
https://issues.apache.org/jira/browse/AVRO-3475?focusedWorklogId=749845&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-749845
]
ASF GitHub Bot logged work on AVRO-3475:
----------------------------------------
Author: ASF GitHub Bot
Created on: 30/Mar/22 05:20
Start Date: 30/Mar/22 05:20
Worklog Time Spent: 10m
Work Description: KyleSchoonover commented on a change in pull request
#1626:
URL: https://github.com/apache/avro/pull/1626#discussion_r838127734
##########
File path: lang/csharp/src/apache/main/Util/TimeMicrosecond.cs
##########
@@ -51,16 +51,29 @@ public override object ConvertToBaseValue(object
logicalValue, LogicalSchema sch
{
var time = (TimeSpan)logicalValue;
- if (time >= _exclusiveUpperBound)
- throw new ArgumentOutOfRangeException(nameof(logicalValue), "A
'time-micros' value can only have the range '00:00:00' to '23:59:59'.");
+ ThrowIfOutOfRange(time, nameof(logicalValue));
+ // Note: UnixEpochDateTime.TimeOfDay is '00:00:00'. This could be
'return time.Ticks / _ticksPerMicrosecond';
return (time - UnixEpochDateTime.TimeOfDay).Ticks /
_ticksPerMicrosecond;
}
/// <inheritdoc/>
public override object ConvertToLogicalValue(object baseValue,
LogicalSchema schema)
{
- return
UnixEpochDateTime.TimeOfDay.Add(TimeSpan.FromTicks((long)baseValue *
_ticksPerMicrosecond));
+ var time = TimeSpan.FromTicks((long)baseValue *
_ticksPerMicrosecond);
+
+ ThrowIfOutOfRange(time, nameof(baseValue));
+
+ // Note: UnixEpochDateTime.TimeOfDay is '00:00:00', so the Add is
meaningless. This could be 'return time;'
+ return UnixEpochDateTime.TimeOfDay.Add(time);
+ }
+
+ private static void ThrowIfOutOfRange(TimeSpan time, string paramName)
+ {
+ if (time.Ticks < 0 || time >= _exclusiveUpperBound)
+ {
+ throw new ArgumentOutOfRangeException(paramName, $"A
'{LogicalTypeName}' value must be at least '00:00:00' and less than
'1.00:00:00'.");
Review comment:
`throw new ArgumentOutOfRangeException(paramName, $"A
'{LogicalTypeName}' value must be at least '{TimeSpan.Zero}' and less than
'{_exclusiveUpperBound}'.");`
Use the constants when possible for messaging.
##########
File path: lang/csharp/src/apache/main/Util/TimeMillisecond.cs
##########
@@ -50,17 +50,29 @@ public override object ConvertToBaseValue(object
logicalValue, LogicalSchema sch
{
var time = (TimeSpan)logicalValue;
- if (time >= _exclusiveUpperBound)
- throw new ArgumentOutOfRangeException(nameof(logicalValue), "A
'time-millis' value can only have the range '00:00:00' to '23:59:59'.");
+ ThrowIfOutOfRange(time, nameof(logicalValue));
+ // Note: UnixEpochDateTime.TimeOfDay is '00:00:00'. This could be
'return time.TotalMilliseconds;
return (int)(time - UnixEpochDateTime.TimeOfDay).TotalMilliseconds;
}
/// <inheritdoc/>
public override object ConvertToLogicalValue(object baseValue,
LogicalSchema schema)
{
- var noMs = (int)baseValue;
- return
UnixEpochDateTime.TimeOfDay.Add(TimeSpan.FromMilliseconds(noMs));
+ var time = TimeSpan.FromMilliseconds((int)baseValue);
+
+ ThrowIfOutOfRange(time, nameof(baseValue));
+
+ // Note: UnixEpochDateTime.TimeOfDay is '00:00:00'. This could be
'return time;'
+ return UnixEpochDateTime.TimeOfDay.Add(time);
+ }
+
+ private static void ThrowIfOutOfRange(TimeSpan time, string paramName)
+ {
+ if (time.Ticks < 0 || time >= _exclusiveUpperBound)
+ {
+ throw new ArgumentOutOfRangeException(paramName, $"A
'{LogicalTypeName}' value must be at least '00:00:00' and less than
'1.00:00:00'.");
Review comment:
`throw new ArgumentOutOfRangeException(paramName, $"A
'{LogicalTypeName}' value must be at least '{TimeSpan.Zero}' and less than
'{_exclusiveUpperBound}'.");`
Use the constants when possible for messaging.
--
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]
Issue Time Tracking
-------------------
Worklog Id: (was: 749845)
Remaining Estimate: 23h 20m (was: 23.5h)
Time Spent: 40m (was: 0.5h)
> Enforce time-millis and time-micros specification
> -------------------------------------------------
>
> Key: AVRO-3475
> URL: https://issues.apache.org/jira/browse/AVRO-3475
> Project: Apache Avro
> Issue Type: Improvement
> Components: csharp
> Reporter: Zoltan Csizmadia
> Priority: Minor
> Labels: pull-request-available
> Fix For: 1.12.0
>
> Original Estimate: 24h
> Time Spent: 40m
> Remaining Estimate: 23h 20m
>
> During fixing AVRO-3471, I noticed that negative in/long values or negative
> TimeSpan values are allowed for time-millis and time-micros.
> [https://avro.apache.org/docs/current/spec.html] states that, it is supposed
> to be the number of us/ms after midnight 00:00:00. The following TimSpan
> values do not cause any overflow exceptions:
>
> "-00:00:00.001"
> "-999999.00:00:00"
>
> # This change should throw exceptions in cases where the code has not thrown
> exceptions before
> # OutOfRange exception message should be changed to `'time-millis/micros'
> value must be at least '00:00:00' and less than '1.00:00:00'.` to reflect the
> spec precisely
>
> It might make sense to introduce this change only in 1.12.0, because of all
> the above changes
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
