[ https://issues.apache.org/jira/browse/AVRO-1667?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15183736#comment-15183736 ]
Zoltan Farkas commented on AVRO-1667: ------------------------------------- Ryan, So far I think your code is logically correct and an improvement on my implementation, the copyFixups method can be improved to: {code} private static void copyFixups(List<Fixup> fixups, Symbol[] out, int outPos, Symbol[] toCopy) { final int nrFixups = fixups.size(); for (int i = 0; i < nrFixups; i++) { Fixup fixup = fixups.get(i); if (fixup.symbols == toCopy) { fixups.add(new Fixup(out, fixup.pos + outPos)); } } } {code} the original implementation was iterating over the added Fixups (since mixups.size() was reevaluated at every iteration) which is unnecessary... > Parser symbol tree flattening is broken for recursive schemas > ------------------------------------------------------------- > > Key: AVRO-1667 > URL: https://issues.apache.org/jira/browse/AVRO-1667 > Project: Avro > Issue Type: Bug > Affects Versions: 1.7.7 > Reporter: Zoltan Farkas > Attachments: AVRO-1667.2.patch, avro-1667.patch > > > Here is a unit test to reproduce: > {noformat} > package org.apache.avro.io.parsing; > import java.io.IOException; > import java.util.HashMap; > import java.util.HashSet; > import java.util.Set; > import junit.framework.Assert; > import org.apache.avro.Schema; > import org.junit.Test; > public class SymbolTest { > private static final String SCHEMA = > "{\"type\":\"record\",\"name\":\"SampleNode\"," > + "\"namespace\":\"org.spf4j.ssdump2.avro\",\n" + > " \"fields\":[\n" + > " {\"name\":\"count\",\"type\":\"int\",\"default\":0},\n" + > " {\"name\":\"subNodes\",\"type\":\n" + > " {\"type\":\"array\",\"items\":{\n" + > " \"type\":\"record\",\"name\":\"SamplePair\",\n" + > " \"fields\":[\n" + > " {\"name\":\"method\",\"type\":\n" + > " {\"type\":\"record\",\"name\":\"Method\",\n" + > " \"fields\":[\n" + > " > {\"name\":\"declaringClass\",\"type\":{\"type\":\"string\",\"avro.java.string\":\"String\"}},\n" > + > " > {\"name\":\"methodName\",\"type\":{\"type\":\"string\",\"avro.java.string\":\"String\"}}\n" > + > " ]}},\n" + > " {\"name\":\"node\",\"type\":\"SampleNode\"}]}}}]}"; > @Test > public void testSomeMethod() throws IOException { > Schema schema = new Schema.Parser().parse(SCHEMA); > Symbol root = Symbol.root(new ResolvingGrammarGenerator() > .generate(schema, schema, new > HashMap<ValidatingGrammarGenerator.LitS, Symbol>())); > validateNonNull(root, new HashSet<Symbol>()); > } > private static void validateNonNull(final Symbol symb, Set<Symbol> seen) { > if (seen.contains(symb)) { > return; > } else { > seen.add(symb); > } > if (symb.production != null) { > for (Symbol s : symb.production) { > if (s == null) { > Assert.fail("invalid parsing tree should not contain > nulls"); > } > if (s.kind != Symbol.Kind.ROOT) { > validateNonNull(s, seen);; > } > } > } > } > } > {noformat} -- This message was sent by Atlassian JIRA (v6.3.4#6332)