Copilot commented on code in PR #3475:
URL: https://github.com/apache/avro/pull/3475#discussion_r2369571192
##########
lang/c++/impl/Compiler.cc:
##########
@@ -638,4 +646,23 @@ AVRO_DECL bool compileJsonSchema(std::istream &is,
ValidSchema &schema, string &
}
}
+AVRO_DECL ValidSchema compileJsonSchemaWithNamedReferences(std::istream &is,
+ const
std::map<Name, ValidSchema> &namedReferences) {
Review Comment:
The function parameter `namedReferences` should be passed by const reference
to avoid unnecessary copying of the map, which could be expensive for large
schemas. Consider using `const std::map<Name, ValidSchema>&` instead.
```suggestion
const
std::map<Name, ValidSchema>& namedReferences) {
```
##########
lang/c++/impl/Compiler.cc:
##########
@@ -94,7 +94,15 @@ static NodePtr makeNode(const string &t, SymbolTable &st,
const string &ns) {
auto it = st.find(n);
if (it != st.end()) {
- return NodePtr(new NodeSymbolic(asSingleAttribute(n), it->second));
+ // Return the raw NodePtr instead of creating a new "NodeSymbolic"
+ // via "NodePtr(new NodeSymbolic(asSingleAttribute(n), it->second))"
+ // in order to support externally resolved named references.
+ // This is safe because the validator canonicalizes duplicates:
+ // when it sees the same named node again (including self-recursion),
+ // it replaces that leaf with a NodeSymbolic via "setLeafToSymbolic".
+ // So even if the raw NodePtr is returned initially, validation
+ // converts repeats to symbolic links.
Review Comment:
[nitpick] The comment explains the rationale but could be clearer about when
this behavior change applies. Consider adding a note that this change affects
both external references and self-references within the same schema compilation.
--
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]
