Re: [PR] AVRO-2825: [csharp] Resolve: C# Logical Types throw exception on unknown logical type [avro]

["KalleOlaviNiemitalo (via GitHub)" <gi...@apache.org>]

KalleOlaviNiemitalo commented on code in PR #2741:
URL: https://github.com/apache/avro/pull/2741#discussion_r1492766807


##########
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");

Review Comment:
   This is not related to logical types.  Please move to a separate pull request.



##########
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 { }
+                        

Review Comment:
   Can LogicalSchema.NewInstance instead be made to work with unrecognized logical types?  For these purposes:
   
   - Avoid swallowing any exceptions thrown for other reasons.
   - Allow applications to parse a schema and read the name of the logical type from LogicalSchema.LogicalTypeName (or even LogicalType.Name) regardless of whether the library recognizes it.
   - Allow applications to parse a schema and serialize it back to JSON without losing the unrecognized logical types here: <https://github.com/apache/avro/blob/49e619b6659d713a1f663ab02bb74439669f3ac8/lang/csharp/src/apache/main/Schema/Property.cs#L33> <https://github.com/apache/avro/blob/49e619b6659d713a1f663ab02bb74439669f3ac8/lang/csharp/src/apache/main/Schema/Property.cs#L45-L46>
   
   LogicalSchema.LogicalType could then be null, or perhaps an instance of a new NotSupportedLogicalType class that would pass everything through without conversion.



-- 
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: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org