wgtmac commented on code in PR #3326:
URL: https://github.com/apache/avro/pull/3326#discussion_r1977101192
##########
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:
The spec does not have a statement for custom logical type but at least the
Java impl is supporting this. I'm not sure whether this is by purpose and it
might be a larger topic to add this to the spec.
##########
lang/c++/impl/LogicalType.cc:
##########
@@ -22,7 +22,10 @@
namespace avro {
LogicalType::LogicalType(Type type)
- : type_(type), precision_(0), scale_(0) {}
+ : type_(type), precision_(0), scale_(0), custom_(nullptr) {}
Review Comment:
That sounds good!
Just want to mention that `LogicalType` has `setPrecision()` and
`setScale()` member functions that break its immutability.
##########
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:
You're right. I have fixed those toJson functions as well.
##########
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:
The current implementation aligns with `LogicalType::printJson` without the
depth parameter.
##########
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:
No, I think usually the registration should only happen when the application
starts. But I agree that it is better to make it thread safe.
--
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]
