AVRO-4026: [c++] Add new methods to CustomAttributes to allow non-string custom attributes in schemas

[jhump]

This adds new methods to CustomAttributes to allow setting non-string values. These other methods work with JSON-encoded strings.

Unlike #3064 and #3069, this change attempts to be backwards compatible. However, from reading more comments in pull requests, it looks like the "fix" I added (to escape the keys and values in custom attributes when printing to JSON) may actually be a compatibility issue since it seems that users were expected to have to escape string values if they contained any characters that would be escaped in JSON (including quotes). That seems like a really terrible API, and it also meant that the values would not round-trip correctly: reading a data file would not create custom attributes with these strings properly escaped, so later writing out data with the same schema would generate an invalid schema JSON document.

In any event, this uses strings as the values even though it would be ideal if we could pass some sort of structured data as the value type. The ideal types (json::Entity and its accompanying json::Object and json::Array types) are defined in json/JsonDom.hh. But that header files is not part of the Avro include files distribution, which means we cannot #include it from CustomAttributes.hh, so it's a no-go. From a little history spelunking, I see that they indeed used to use a structured form which was simplified to strings in #1821, purely because these JSON header files aren't available to users in the Avro distribution.

Alternatives that I considered for using JSON-encoded strings:

• Include the JSON types in the Avro distribution. I think this would be as simple as moving the header files out of lang/c++/impl and into lang/c++/include/avro. But then we expand the public API of too much. This approach was already tried and rejected in AVRO-3601: C++ API header contains breaking include #1820.
• Use a wrapper like std::any as the value type. This can be #included in CustomAttributes.hh but eliminates type safety in the signature. The only concrete accepted value would likely be json::Entity -- though we could make it more sophisticated and also allow the various value types sans wrapper: std::string, bool, int64_t, double, json::Array (aka std::vector<json::Entity>), and json::Object (aka std::map<std::string, json::Entity>). But this isn't really usable by external/user code, at least not for any composite values, since they aren't able to include the JSON headers and then produce valid values of json::Entity.
• Convert from json::Entity to some other structured type that is defined in CustomAttributes.hh. This could possibly be a concrete std::variant that allows the various options (and could use std::monostate to represent JSON null values). This introduces non-trivial conversion code. From a performance perspective, it could likely be better than converting to/from strings, but it's a non-trivial amount of new-new code to maintain, which didn't feel right.

What is the purpose of the change

Fixes exception from C++ library when compiling schemas with non-string custom attributes.

Verifying this change

This change added tests and can be verified as follows:

• Updates existing unit test cases to use the new method and use non-string custom attributes.

Documentation

• Does this pull request introduce a new feature? yes
• If yes, how is the feature documented? not documented

[martin-g]

@wgtmac Do you have an opinion about these changes ?
There are two more PR about the same (#3064 and #3069). We try to find the best one of them.

[wgtmac]

I'm not fully familiar with the context (and the code) yet. I agree that keeping backward compatibility is the bottom line. However, from the JIRA issue it seems that non-string attributes were supported once but broken after fixing other issues. Shouldn't we just fix it to revert the unexpected behavior? Except the alternatives list above, is it possible to add an option to CustomAttributes to control its behavior of dealing with non-string values?

[jhump]

Except the alternatives list above, is it possible to add an option to CustomAttributes to control its behavior of dealing with non-string values?

Most certainly. I hadn't considered it, but it would be easy to change to that. Would like you me to apply that suggestion in this PR? For back-compat, the "default" mode would have to be the awkward (arguably broken) string-only mode, that just wraps the input in double-quotes (with no other escaping). But a new constructor could be added to enable a mode where the API expects well-formed JSON-encoded data (so string values must be quoted and escaped).