-
Notifications
You must be signed in to change notification settings - Fork 1.8k
AVRO-2825: [csharp] Resolve: C# Logical Types throw exception on unkn… #2751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
ad61af0
e5a7ded
8909529
235d329
f7b4872
1703064
60463b5
42b0fbd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "profiles": { | ||
| "Avro.codegen": { | ||
| "commandName": "Project", | ||
| "commandLineArgs": "-s C:\\Users\\Thomas.Bruns\\source\\repos\\AzureDevOps\\TQL.Kafka.Samples\\TQL.Kafka.Samples\\TQL.Kafka.Samples.Messages\\v2\\cdc_customers_dbo_tblcustomers.avsc .\\tab" | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "profiles": { | ||
| "Avro.main": { | ||
| "commandName": "Project", | ||
| "commandLineArgs": "C:\\Users\\Thomas.Bruns\\source\\repos\\AzureDevOps\\TQL.Kafka.Samples\\TQL.Kafka.Samples\\TQL.Kafka.Samples.Messages\\v2\\cdc_customers_dbo_tblcustomers.avsc" | ||
|
TomBruns marked this conversation as resolved.
Outdated
|
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,7 +45,7 @@ private LogicalTypeFactory() | |
| { TimeMicrosecond.LogicalTypeName, new TimeMicrosecond() }, | ||
| { TimestampMillisecond.LogicalTypeName, new TimestampMillisecond() }, | ||
| { TimestampMicrosecond.LogicalTypeName, new TimestampMicrosecond() }, | ||
| { Uuid.LogicalTypeName, new Uuid() } | ||
| { Uuid.LogicalTypeName, new Uuid() }, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove comma plz |
||
| }; | ||
| } | ||
|
|
||
|
|
@@ -67,22 +67,22 @@ public void Register(LogicalType logicalType) | |
| /// <returns>A <see cref="LogicalType" />.</returns> | ||
| public LogicalType GetFromLogicalSchema(LogicalSchema schema, bool ignoreInvalidOrUnknown = false) | ||
| { | ||
| try | ||
| { | ||
| if (!_logicalTypes.TryGetValue(schema.LogicalTypeName, out LogicalType logicalType)) | ||
| throw new AvroTypeException("Logical type '" + schema.LogicalTypeName + "' is not supported."); | ||
| LogicalType logicalType = null; | ||
|
|
||
| if (_logicalTypes.TryGetValue(schema.LogicalTypeName, out logicalType)) | ||
| { | ||
| logicalType.ValidateSchema(schema); | ||
|
|
||
| return logicalType; | ||
| } | ||
| catch (AvroTypeException) | ||
| else if (ignoreInvalidOrUnknown) | ||
| { | ||
| logicalType = new UnknownLogicalType(schema); | ||
| } | ||
| else | ||
| { | ||
| if (!ignoreInvalidOrUnknown) | ||
| throw; | ||
| throw new AvroTypeException("Logical type '" + schema.LogicalTypeName + "' is not supported."); | ||
| } | ||
|
|
||
| return null; | ||
| return logicalType; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Text; | ||
|
|
||
| namespace Avro.Util | ||
| { | ||
| public class UnknownLogicalType : LogicalType | ||
| { | ||
| public LogicalSchema Schema { get; } | ||
|
|
||
| public UnknownLogicalType(LogicalSchema schema) : base(schema.LogicalTypeName) | ||
| { | ||
| this.Schema = schema; | ||
| } | ||
|
|
||
| public override object ConvertToBaseValue(object logicalValue, LogicalSchema schema) | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
Comment on lines
+52
to
+73
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These casts are almost no-ops, because the result of each cast is immediately converted back to The casts have two effects, but I don't think those effects matter here, because the logical value should be the same as the base value, which should be an instance of one of the Avro standard types:
|
||
|
|
||
| public override object ConvertToLogicalValue(object baseValue, LogicalSchema schema) | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| public override Type GetCSharpType(bool nullible) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perhaps you meant nullable instead of nullible?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with your comment but that name pre-existed in many files in this codebase so I choose to be consistent. |
||
| { | ||
| // handle all Primitive Types | ||
| switch (this.Schema.BaseSchema.Name) | ||
| { | ||
| case @"string": | ||
| return typeof(System.String); | ||
| case @"boolean": | ||
| return typeof(System.Boolean); | ||
| case @"int": | ||
| return typeof(System.Int32); | ||
| case @"long": | ||
| return typeof(System.Int64); | ||
| case @"float": | ||
| return typeof(System.Single); | ||
| case @"double": | ||
| return typeof(System.Double); | ||
| case @"bytes": | ||
| return typeof(System.Byte[]); | ||
| default: | ||
| return typeof(System.Object); | ||
| } | ||
| } | ||
|
|
||
| public override bool IsInstanceOfLogicalType(object logicalValue) | ||
| { | ||
| // => throw new NotImplementedException(); | ||
| return true; | ||
| } | ||
|
|
||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.