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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
