Repository: hamba/avro
Pull request #443 (2024-09-06 17:07:30)

fix: Long timestamp default decoding
#368 related

The previous fix made a restriction to prevent accepting `int64` as a Go type for the Long-timestamp codec. However, this restriction affected the `defaultDecoder` when dealing with the JSON-decoded default value represented as `int64`.

A solution could be:
1 - Improve the default JSON decoding to ensure that the default value is correctly represented as `time.Time`.
2- Relax the restriction made in the previous fix and limit it to the case of `time.Duration`.


The PR draft presents these solutions, and a test has been added to cover this case.



redaLaanait: Thinking about #368, the fix seems to me overly restrictive and beyond the problem’s scope:

_The accidental case of encoding a time.Duration value (which is practically in nano second) as timestamp in milliseconds._

IMHO, that’s a misuse of GO types and not a bug in the library’s implementation of AVRO spec.
nrwiersma: You are likely right that it should be more specific to only restricting the `time.Duration` case. I will look deeper into this this weekend, when I have more mental bandwidth. Good catch.
nrwiersma: I agree with the approach. This looks like a good fix.

Patch:
diff --git a/codec.go b/codec.go
index 92c0991..fc6706e 100644
--- a/codec.go
+++ b/codec.go
@@ -11,9 +11,10 @@ import (
 )
 
 var (
-	timeType = reflect.TypeOf(time.Time{})
-	ratType  = reflect.TypeOf(big.Rat{})
-	durType  = reflect.TypeOf(LogicalDuration{})
+	timeType         = reflect.TypeOf(time.Time{})
+	timeDurationType = reflect.TypeOf(time.Duration(0))
+	ratType          = reflect.TypeOf(big.Rat{})
+	durType          = reflect.TypeOf(LogicalDuration{})
 )
 
 type null struct{}
diff --git a/codec_native.go b/codec_native.go
index e4c5a41..675de08 100644
--- a/codec_native.go
+++ b/codec_native.go
@@ -83,16 +83,17 @@ func createDecoderOfNative(schema *PrimitiveSchema, typ reflect2.Type) ValDecode
 				convert: createLongConverter(schema.encodedType),
 			}
 
-		case st == Long && lt == "":
+		case st == Long:
+			isTimestamp := (lt == TimestampMillis || lt == TimestampMicros)
+			if isTimestamp && typ.Type1() == timeDurationType {
+				return &errorDecoder{err: fmt.Errorf("avro: %s is unsupported for Avro %s and logicalType %s",
+					typ.Type1().String(), schema.Type(), lt)}
+			}
 			if resolved {
 				return &longConvCodec[int64]{convert: createLongConverter(schema.encodedType)}
 			}
 			return &longCodec[int64]{}
 
-		case lt != "":
-			return &errorDecoder{err: fmt.Errorf("avro: %s is unsupported for Avro %s and logicalType %s",
-				typ.String(), schema.Type(), lt)}
-
 		default:
 			break
 		}
@@ -245,13 +246,14 @@ func createEncoderOfNative(schema Schema, typ reflect2.Type) ValEncoder {
 		case st == Long && lt == TimeMicros: // time.Duration
 			return &timeMicrosCodec{}
 
-		case st == Long && lt == "":
+		case st == Long:
+			isTimestamp := (lt == TimestampMillis || lt == TimestampMicros)
+			if isTimestamp && typ.Type1() == timeDurationType {
+				return &errorEncoder{err: fmt.Errorf("avro: %s is unsupported for Avro %s and logicalType %s",
+					typ.Type1().String(), schema.Type(), lt)}
+			}
 			return &longCodec[int64]{}
 
-		case lt != "":
-			return &errorEncoder{err: fmt.Errorf("avro: %s is unsupported for Avro %s and logicalType %s",
-				typ.String(), schema.Type(), lt)}
-
 		default:
 			break
 		}


Test patch:
diff --git a/schema_compatibility_test.go b/schema_compatibility_test.go
index 5b4ca13..83485cd 100644
--- a/schema_compatibility_test.go
+++ b/schema_compatibility_test.go
@@ -2,6 +2,7 @@ package avro_test
 
 import (
 	"math/big"
+	"strconv"
 	"testing"
 	"time"
 
@@ -815,6 +816,62 @@ func TestSchemaCompatibility_Resolve(t *testing.T) {
 				"b": map[string]any{"a": int64(20)},
 			},
 		},
+		{
+			name: "Record Writer Field Missing With Long timestamp-millis Default",
+			reader: `{
+						"type":"record", "name":"test", "namespace": "org.hamba.avro", 
+						"fields":[
+							{"name": "a", "type": "string"},
+							{
+								"name": "b",
+								"type": {
+									"type": "long", 
+									"logicalType": "timestamp-millis"
+								}, 
+								"default": ` + strconv.FormatInt(1725616800000, 10) + `
+							}
+						]
+					}`,
+			writer: `{
+						"type":"record", "name":"test", "namespace": "org.hamba.avro", 
+						"fields":[
+							{"name": "a", "type": "string"}
+						]
+					}`,
+			value: map[string]any{"a": "foo"},
+			want: map[string]any{
+				"a": "foo",
+				"b": time.UnixMilli(1725616800000).UTC(), // 2024-09-06 10:00:00
+			},
+		},
+		{
+			name: "Record Writer Field Missing With Long timestamp-micros Default",
+			reader: `{
+						"type":"record", "name":"test", "namespace": "org.hamba.avro", 
+						"fields":[
+							{"name": "a", "type": "string"},
+							{
+								"name": "b",
+								"type": {
+									"type": "long", 
+									"logicalType": "timestamp-micros"
+								}, 
+								"default": ` + strconv.FormatInt(1725616800000000, 10) + `
+							}
+						]
+					}`,
+			writer: `{
+						"type":"record", "name":"test", "namespace": "org.hamba.avro", 
+						"fields":[
+							{"name": "a", "type": "string"}
+						]
+					}`,
+			value: map[string]any{"a": "foo"},
+			want: map[string]any{
+				"a": "foo",
+				"b": time.UnixMicro(1725616800000000).UTC(), // 2024-09-06 10:00:00
+			},
+		},
 	}
 
 	for _, test := range tests {