From 28d4c901c88bb40b90e8f4d77c68c6d65383ac24 Mon Sep 17 00:00:00 2001 From: Tom Bruns Date: Fri, 16 Feb 2024 07:42:40 -0500 Subject: [PATCH 1/4] Per AVRO Spec (v1.8.0 - v1.11.1) ... Logical Types Section Language implementations must ignore unknown logical types when reading, and should use the underlying Avro type. --- lang/csharp/src/apache/main/Schema/Schema.cs | 11 +++- .../apache/test/AvroGen/AvroGenSchemaTests.cs | 4 +- .../src/apache/test/Schema/SchemaTests.cs | 12 +++- .../src/apache/test/Util/LogicalTypeTests.cs | 55 +++++++++++++++++++ 4 files changed, 76 insertions(+), 6 deletions(-) diff --git a/lang/csharp/src/apache/main/Schema/Schema.cs b/lang/csharp/src/apache/main/Schema/Schema.cs index 3e54653f015..6960f1d25e0 100644 --- a/lang/csharp/src/apache/main/Schema/Schema.cs +++ b/lang/csharp/src/apache/main/Schema/Schema.cs @@ -192,9 +192,14 @@ internal static Schema ParseJson(JToken jtok, SchemaNames names, string encspace return ArraySchema.NewInstance(jtok, props, names, encspace); if (type.Equals("map", StringComparison.Ordinal)) return MapSchema.NewInstance(jtok, props, names, encspace); - if (null != jo["logicalType"]) // logical type based on a primitive - return LogicalSchema.NewInstance(jtok, props, names, encspace); - + try + { + if (null != jo["logicalType"]) // logical type based on a primitive + return LogicalSchema.NewInstance(jtok, props, names, encspace); + } + // swallow exception from unknown logicalType + catch { } + Schema schema = PrimitiveSchema.NewInstance((string)type, props); if (null != schema) return schema; diff --git a/lang/csharp/src/apache/test/AvroGen/AvroGenSchemaTests.cs b/lang/csharp/src/apache/test/AvroGen/AvroGenSchemaTests.cs index 807acbda92a..3c976b6a2d3 100644 --- a/lang/csharp/src/apache/test/AvroGen/AvroGenSchemaTests.cs +++ b/lang/csharp/src/apache/test/AvroGen/AvroGenSchemaTests.cs @@ -619,7 +619,9 @@ public void NotSupportedSchema(string schema, Type expectedException) string schemaFileName = Path.Combine(outputDir, $"{uniqueId}.avsc"); System.IO.File.WriteAllText(schemaFileName, schema); - Assert.That(AvroGenTool.GenSchema(schemaFileName, outputDir, new Dictionary(), false), Is.EqualTo(1)); + // Note: Unknown logical types are now ignored + //Assert.That(AvroGenTool.GenSchema(schemaFileName, outputDir, new Dictionary(), false), Is.EqualTo(1)); + Assert.That(AvroGenTool.GenSchema(schemaFileName, outputDir, new Dictionary(), false), Is.EqualTo(0)); } finally { diff --git a/lang/csharp/src/apache/test/Schema/SchemaTests.cs b/lang/csharp/src/apache/test/Schema/SchemaTests.cs index 319e9a95be3..12b768d2b7d 100644 --- a/lang/csharp/src/apache/test/Schema/SchemaTests.cs +++ b/lang/csharp/src/apache/test/Schema/SchemaTests.cs @@ -548,12 +548,20 @@ public void TestLogicalPrimitive(string s, string baseType, string logicalType) testToString(sc); } + // Per AVRO Spec (v1.8.0 - v1.11.1) ... Logical Types Section + // Language implementations must ignore unknown logical types when reading, and should use the underlying Avro type. [TestCase("{\"type\": \"int\", \"logicalType\": \"unknown\"}", "unknown")] public void TestUnknownLogical(string s, string unknownType) { - var err = Assert.Throws(() => Schema.Parse(s)); + //var err = Assert.Throws(() => Schema.Parse(s)); + Assert.DoesNotThrow(() => + { + var schema = Schema.Parse(s); + // confirm that the schema defaults to the underlying type + Assert.IsTrue(schema.Name == @"int"); + }); - Assert.AreEqual("Logical type '" + unknownType + "' is not supported.", err.Message); + //Assert.AreEqual("Logical type '" + unknownType + "' is not supported.", err.Message); } [TestCase("{\"type\": \"map\", \"values\": \"long\"}", "long")] diff --git a/lang/csharp/src/apache/test/Util/LogicalTypeTests.cs b/lang/csharp/src/apache/test/Util/LogicalTypeTests.cs index 0129b2a5b45..be47228ad78 100644 --- a/lang/csharp/src/apache/test/Util/LogicalTypeTests.cs +++ b/lang/csharp/src/apache/test/Util/LogicalTypeTests.cs @@ -17,7 +17,9 @@ */ using System; +using System.Composition.Convention; using System.Globalization; +using System.Linq; using System.Numerics; using Avro.Util; using NUnit.Framework; @@ -419,5 +421,58 @@ public void TestUuid(string guidString) var converted = (Guid) avroUuid.ConvertToLogicalValue(avroUuid.ConvertToBaseValue(guid, schema), schema); Assert.AreEqual(guid, converted); } + + /* + { + "fields": [ + { + "default": 0, + "name": "firstField", + "type": "int" + }, + { + "default": null, + "name": "secondField", + "type": [ + "null", + { + "logicalType": "varchar", + "maxLength": 65, + "type": "string" + } + ] + } + ], + "name": "sample_schema", + "type": "record" + } + */ + + // Before Change will throw Avro.AvroTypeException: 'Logical type 'varchar' is not supported.' + // Per AVRO Spec (v1.8.0 - v1.11.1) ... Logical Types Section + // Language implementations must ignore unknown logical types when reading, and should use the underlying Avro type. + [TestCase("{\"fields\": [{\"default\": 0,\"name\": \"firstField\",\"type\": \"int\"},{\"default\": null,\"name\": \"secondField\",\"type\": [\"null\",{\"logicalType\": \"varchar\",\"maxLength\": 65,\"type\": \"string\"}]}],\"name\": \"sample_schema\",\"type\": \"record\"}")] + public void TestUnknownLogicalType(string schemaText) + { + var schema = Avro.Schema.Parse(schemaText); + Assert.IsNotNull(schema); + + var secondField = ((RecordSchema)schema).Fields.FirstOrDefault(f => f.Name == @"secondField"); + Assert.IsNotNull(secondField); + + var secondFieldSchema = ((Field)secondField).Schema; + Assert.IsNotNull(secondFieldSchema); + + var secondFieldUnionSchema = (UnionSchema)secondFieldSchema; + Assert.IsNotNull(secondFieldUnionSchema); + + var props = secondFieldUnionSchema.Schemas.Where(s => s.Props != null).ToList(); + Assert.IsNotNull(props); + Assert.IsTrue(props.Count == 1); + + var prop = props[0]; + // Confirm that the unknown logical type is ignored and the underlying AVRO type is used + Assert.IsTrue(prop.Name == @"string"); + } } } From c9c1bed650df60414ffa59a450a2f08c47ce0468 Mon Sep 17 00:00:00 2001 From: Tom Bruns Date: Fri, 16 Feb 2024 08:32:44 -0500 Subject: [PATCH 2/4] AVRO-2825 C# Logical Types throw exception on unknown logical type also resolved exception thrown if namespace is missing from schema --- lang/csharp/src/apache/main/CodeGen/CodeGen.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs index 7e793627201..ce0014916cb 100644 --- a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs +++ b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs @@ -893,13 +893,14 @@ protected virtual CodeTypeDeclaration processRecord(Schema schema) ctd.Members.Add(cmmPut); string nspace = recordSchema.Namespace; - if (string.IsNullOrEmpty(nspace)) - { - throw new CodeGenException("Namespace required for record schema " + recordSchema.Name); - } - - CodeNamespace codens = AddNamespace(nspace); - + //if (string.IsNullOrEmpty(nspace)) + //{ + // throw new CodeGenException("Namespace required for record schema " + recordSchema.Name); + //} + + // AVRO spec DOES NOT require a Namespace but this code does. + // Workaround is to inject a fixed string that will be obvious if required + CodeNamespace codens = (!string.IsNullOrEmpty(nspace)) ? AddNamespace(nspace) : AddNamespace(@"SchemaHadNoNamespace"); codens.Types.Add(ctd); return ctd; From 3f75a9ae52e5cb01e3751cdc2eb53b895e73bef2 Mon Sep 17 00:00:00 2001 From: Tom Bruns Date: Fri, 16 Feb 2024 08:43:06 -0500 Subject: [PATCH 3/4] Revert "AVRO-2825 C# Logical Types throw exception on unknown logical type" This reverts commit c9c1bed650df60414ffa59a450a2f08c47ce0468. --- lang/csharp/src/apache/main/CodeGen/CodeGen.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs index ce0014916cb..7e793627201 100644 --- a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs +++ b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs @@ -893,14 +893,13 @@ protected virtual CodeTypeDeclaration processRecord(Schema schema) ctd.Members.Add(cmmPut); string nspace = recordSchema.Namespace; - //if (string.IsNullOrEmpty(nspace)) - //{ - // throw new CodeGenException("Namespace required for record schema " + recordSchema.Name); - //} - - // AVRO spec DOES NOT require a Namespace but this code does. - // Workaround is to inject a fixed string that will be obvious if required - CodeNamespace codens = (!string.IsNullOrEmpty(nspace)) ? AddNamespace(nspace) : AddNamespace(@"SchemaHadNoNamespace"); + if (string.IsNullOrEmpty(nspace)) + { + throw new CodeGenException("Namespace required for record schema " + recordSchema.Name); + } + + CodeNamespace codens = AddNamespace(nspace); + codens.Types.Add(ctd); return ctd; From 556cff3aea755315aa038f74224d0ac57b04dbe9 Mon Sep 17 00:00:00 2001 From: Tom Bruns Date: Fri, 16 Feb 2024 09:22:16 -0500 Subject: [PATCH 4/4] Resolve AVRO-3941 C# implementation of avrogen throws an exception if the schema file does not contain a namespace This reverts commit 3f75a9ae52e5cb01e3751cdc2eb53b895e73bef2. --- lang/csharp/src/apache/main/CodeGen/CodeGen.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs index 7e793627201..ce0014916cb 100644 --- a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs +++ b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs @@ -893,13 +893,14 @@ protected virtual CodeTypeDeclaration processRecord(Schema schema) ctd.Members.Add(cmmPut); string nspace = recordSchema.Namespace; - if (string.IsNullOrEmpty(nspace)) - { - throw new CodeGenException("Namespace required for record schema " + recordSchema.Name); - } - - CodeNamespace codens = AddNamespace(nspace); - + //if (string.IsNullOrEmpty(nspace)) + //{ + // throw new CodeGenException("Namespace required for record schema " + recordSchema.Name); + //} + + // AVRO spec DOES NOT require a Namespace but this code does. + // Workaround is to inject a fixed string that will be obvious if required + CodeNamespace codens = (!string.IsNullOrEmpty(nspace)) ? AddNamespace(nspace) : AddNamespace(@"SchemaHadNoNamespace"); codens.Types.Add(ctd); return ctd;