martin-g commented on code in PR #3332:
URL: https://github.com/apache/avro/pull/3332#discussion_r1993144711
##########
lang/c++/include/avro/NodeImpl.hh:
##########
@@ -164,15 +164,12 @@ protected:
customAttributes_.add(customAttributes);
}
- CustomAttributes getCustomAttributes() const override {
- CustomAttributes mergedCustomAttributes;
- for (size_t i = 0; i < customAttributes_.size(); i++) {
- const auto &customAttribute = customAttributes_.get(i);
- for (const auto &[key, value] : customAttribute.attributes()) {
- mergedCustomAttributes.addAttribute(key, value);
- }
- }
- return mergedCustomAttributes;
+ size_t customAttributes() const override {
+ return customAttributes_.size();
+ }
+
+ const CustomAttributes &customAttributesAt(size_t index) const override {
+ return customAttributes_.get(index);
Review Comment:
What if the index is out of bounds ?
##########
lang/c++/test/SchemaTests.cc:
##########
@@ -658,6 +659,158 @@ static void testMalformedLogicalTypes(const char *schema)
{
BOOST_CHECK(datum.logicalType().type() == LogicalType::NONE);
}
+static void testParseCustomAttributes() {
+ const std::string schema = R"({
+ "type": "record",
+ "name": "my_record",
+ "fields": [
+ { "name": "long_field",
+ "type": ["null", "long"],
+ "field-id": 1 },
+ { "name": "array_field",
+ "type": { "type": "array", "items": "int", "element-id": 3 },
+ "field-id": 2,
+ "extra": "1", "extra2": "2" },
+ { "name": "map_field",
+ "type": { "type": "map", "values": "int", "key-id": 5,
"value-id": 6 },
+ "field-id": 4,
+ "extra": "foo" },
+ { "name": "timestamp_field",
+ "type": "long", "logicalType": "timestamp-micros",
"adjust-to-utc": true,
+ "field-id": 10,
+ "extra": "bar" }
+ ]
+ })";
+
+ ValidSchema compiledSchema = compileJsonSchemaFromString(schema);
+ const NodePtr &root = compiledSchema.root();
+ BOOST_CHECK_EQUAL(root->customAttributes(), 4);
+
+ // long_field
+ {
+ auto customAttributes = root->customAttributesAt(0);
+ BOOST_CHECK_EQUAL(customAttributes.getAttribute("field-id").value(),
"1");
+ }
+
+ // array_field
+ {
+ auto customAttributes = root->customAttributesAt(1);
+ BOOST_CHECK_EQUAL(customAttributes.getAttribute("extra").value(), "1");
Review Comment:
I find this API weird.
So, if the custom attributes are at the field's root level then you read
them with `root->customAttributesAt(1).getAttribute("extra")`. But if there is
there is something custom in the `type` then you need
https://github.com/apache/avro/pull/3332/files#diff-559b3bdb2519c3e90855b7755c68a914b6507152dd6d1efe4fa77783ad4502c4R702-R705
I don't know what should be the correct behavior though. I think the Rust
SDK just ignores the custom attributes in `type: {...}`
##########
lang/c++/test/SchemaTests.cc:
##########
@@ -658,6 +659,158 @@ static void testMalformedLogicalTypes(const char *schema)
{
BOOST_CHECK(datum.logicalType().type() == LogicalType::NONE);
}
+static void testParseCustomAttributes() {
+ const std::string schema = R"({
+ "type": "record",
+ "name": "my_record",
+ "fields": [
+ { "name": "long_field",
+ "type": ["null", "long"],
+ "field-id": 1 },
+ { "name": "array_field",
+ "type": { "type": "array", "items": "int", "element-id": 3 },
+ "field-id": 2,
+ "extra": "1", "extra2": "2" },
+ { "name": "map_field",
+ "type": { "type": "map", "values": "int", "key-id": 5,
"value-id": 6 },
+ "field-id": 4,
+ "extra": "foo" },
+ { "name": "timestamp_field",
+ "type": "long", "logicalType": "timestamp-micros",
"adjust-to-utc": true,
+ "field-id": 10,
+ "extra": "bar" }
+ ]
+ })";
+
+ ValidSchema compiledSchema = compileJsonSchemaFromString(schema);
+ const NodePtr &root = compiledSchema.root();
+ BOOST_CHECK_EQUAL(root->customAttributes(), 4);
Review Comment:
Please add more random examples. For example a field without custom fields.
I wonder what would be the value of `root->customAttributes()` then
--
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]
