SahilKang commented on code in PR #843:
URL: https://github.com/apache/avro/pull/843#discussion_r1524736765
##########
lang/c/examples/quickstop.c:
##########
@@ -48,98 +48,86 @@ void init_schema(void)
}
}
-/* Create a value to match the person schema and save it */
+/* Create a datum to match the person schema and save it */
Review Comment:
this switches the example code from the "value api" to the legacy "datum api"
##########
lang/c/src/datum.c:
##########
@@ -153,7 +155,8 @@ static avro_datum_t avro_bytes_private(char *bytes, int64_t
size,
return &datum->obj;
}
-avro_datum_t avro_bytes(const char *bytes, int64_t size)
+avro_datum_t avro_bytes(avro_schema_t schema,
+ const char *bytes, int64_t size)
Review Comment:
this breaks backwards compatibility with the legacy api: stragglers that
haven't updated their codebase to the current api probably won't be eager to do
so for the sake of logical schema support
##########
lang/c/src/schema.h:
##########
@@ -76,12 +78,30 @@ struct avro_link_schema_t {
avro_schema_t to;
};
+struct avro_bytes_schema_t {
+ struct avro_obj_t obj;
+ avro_logical_schema_t *logical_type;
+};
Review Comment:
I think to provide "first class" logical schema support, this approach would
need to be reversed: instead of primitive/complex schemas containing logical
schemas, the logical schemas should wrap their underlying types in some sense
by first class support, I mean that
[avro_type_t](https://github.com/apache/avro/blob/78540a1e59d8556226b9ff1484ff17212137de62/lang/c/src/avro/basics.h#L29-L47)
doesn't have any logical schema branches...which means that
[avro_value_iface::get_type](https://github.com/apache/avro/blob/78540a1e59d8556226b9ff1484ff17212137de62/lang/c/src/avro/value.h#L100-L103)
alone wouldn't be enough to determine if a schema is logical
--
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-unsubscr...@avro.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
