KalleOlaviNiemitalo commented on code in PR #3326:
URL: https://github.com/apache/avro/pull/3326#discussion_r1975528313
##########
lang/c++/include/avro/LogicalType.hh:
##########
@@ -57,12 +64,53 @@ public:
void setScale(int32_t scale);
int32_t scale() const { return scale_; }
+ void setCustomLogicalType(std::shared_ptr<CustomLogicalType> custom);
+ const std::shared_ptr<CustomLogicalType> &customLogicalType() const {
+ return custom_;
+ }
+
void printJson(std::ostream &os) const;
private:
Type type_;
int32_t precision_;
int32_t scale_;
+ std::shared_ptr<CustomLogicalType> custom_;
+};
+
+class AVRO_DECL CustomLogicalType {
+public:
+ CustomLogicalType(const std::string &name) : name_(name) {}
+
+ virtual ~CustomLogicalType() = default;
+
+ const std::string &name() const { return name_; }
+
+ virtual void printJson(std::ostream &os) const;
Review Comment:
This could also be given `size_t depth` from NodePrimitive::printJson and
NodeFixed::printJson. So that if this writes multiple lines, it can indent the
subsequent lines properly.
##########
lang/c++/impl/LogicalType.cc:
##########
@@ -92,7 +102,31 @@ void LogicalType::printJson(std::ostream &os) const {
case UUID:
os << R"("logicalType": "uuid")";
break;
+ case CUSTOM:
+ custom_->printJson(os);
+ break;
+ }
+}
+
+void CustomLogicalType::printJson(std::ostream &os) const {
+ os << R"("logicalType": ")" << name_ << "\"";
Review Comment:
This assumes the name of a custom logical type won't contain quotation
marks, backslashes, or control characters not allowed in JSON strings. That is
a reasonable restriction but I feel it should be documented in the Avro
specification.
##########
lang/c++/include/avro/LogicalType.hh:
##########
@@ -57,12 +64,53 @@ public:
void setScale(int32_t scale);
int32_t scale() const { return scale_; }
+ void setCustomLogicalType(std::shared_ptr<CustomLogicalType> custom);
+ const std::shared_ptr<CustomLogicalType> &customLogicalType() const {
+ return custom_;
+ }
+
void printJson(std::ostream &os) const;
private:
Type type_;
int32_t precision_;
int32_t scale_;
+ std::shared_ptr<CustomLogicalType> custom_;
+};
+
+class AVRO_DECL CustomLogicalType {
+public:
+ CustomLogicalType(const std::string &name) : name_(name) {}
+
+ virtual ~CustomLogicalType() = default;
+
+ const std::string &name() const { return name_; }
+
+ virtual void printJson(std::ostream &os) const;
+
+private:
+ std::string name_;
+};
+
+// Registry for custom logical types.
+// This class is not thread-safe.
+class AVRO_DECL CustomLogicalTypeRegistry {
+public:
+ static CustomLogicalTypeRegistry &instance();
Review Comment:
A non-thread-safe singleton seems risky. Does the library already have some
other registry like that?
##########
lang/c++/test/unittest.cc:
##########
@@ -1066,6 +1067,33 @@ void testNestedMapSchema() {
BOOST_CHECK_EQUAL(expected, actual.str());
}
+void testCustomLogicalType() {
+ struct MapLogicalType : public CustomLogicalType {
+ MapLogicalType() : CustomLogicalType("map") {}
+ };
+
+ CustomLogicalTypeRegistry::instance().registerType("map", [](const
std::string &) {
+ return std::make_shared<MapLogicalType>();
+ });
+
+ std::string schema =
+ R"({ "type": "array",
+ "logicalType": "map",
+ "items": {
+ "type": "record",
+ "name": "k12_v13",
+ "fields": [
+ { "name": "key", "type": "int", "field-id": 12 },
+ { "name": "value", "type": "string", "field-id": 13 }
+ ]
+ }
+ })";
+ auto compiledSchema = compileJsonSchemaFromString(schema);
+ auto logicalType = compiledSchema.root()->logicalType();
+ BOOST_CHECK_EQUAL(logicalType.type(), avro::LogicalType::CUSTOM);
+ BOOST_CHECK_EQUAL(logicalType.customLogicalType()->name(), "map");
Review Comment:
Should also write the schema back to a JSON string and verify that the
logical type is included there, perhaps by parsing it a second time. Because
NodeArray::printJson does not call `logicalType().printJson(os)`, I suspect it
doesn't work yet.
--
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]
