jmd-z commented on PR #3069: URL: https://github.com/apache/avro/pull/3069#issuecomment-2377245165
This particular case isn't much of a problem. The current behavior of the C++ library is to throw an exception if you attempt to read a schema with non-string custom attributes, so this is unlikely in the wild. This test creates the schema programmatically bypassing the reading issue. The compatibility issue with this PR is existing code that interrogates the value of a custom attribute that is a string type. With this PR, the value is JSON encoded text, so the returned value will contain a quoted string instead of the raw string in that case. There is another PR 3604 that avoids this compatibility issue, but at the cost of creating ambiguity. The API was recently changed, remaining CustomFields to CustomAttributes, signaling a breaking change. Perhaps renaming the function CustomAttributes::getAttributes to CustomAttributes::getAttributesJSON would do the same here. On Thu, Sep 26, 2024, 7:13 AM Martin Grigorov ***@***.***> wrote: > ***@***.**** commented on this pull request. > ------------------------------ > > In lang/c++/test/unittest.cc > <https://github.com/apache/avro/pull/3069#discussion_r1776855615>: > > > @@ -457,11 +457,11 @@ struct TestSchema { > std::string expectedJsonWithCustomAttribute = > "{\"type\": \"record\", \"name\": \"Test\",\"fields\": " > "[{\"name\": \"f1\", \"type\": \"long\", " > - "\"arrayField\": \"[1]\", " > - "\"booleanField\": \"true\", " > - "\"mapField\": \"{\\\"key1\\\":\\\"value1\\\", \\\"key2\\\":\\\"value2\\\"}\", " > - "\"nullField\": \"null\", " > - "\"numberField\": \"1.23\", " > + "\"arrayField\": [1], " > > Looking at these changes I think this change may break someone's > application. > Do we want to support a backward compatible encoding via some > property/setting/... ? > > — > Reply to this email directly, view it on GitHub > <https://github.com/apache/avro/pull/3069#pullrequestreview-2330889691>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/BKKFEFOTN2DUMQW6AUKEGT3ZYPT7DAVCNFSM6AAAAABMDDFJFGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMZQHA4DSNRZGE> > . > You are receiving this because you authored the thread.Message ID: > ***@***.***> > -- 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]
