I happened to notice that MINVFUNC in 0003 displays like this "fmt": "MINVFUNC==%{type}T", in some cases; this appears in the JSON that's emitted by the regression tests at some point. How can we detect this kind of thing, so that these mistakes become self-evident? I thought the intention of the regress module was to run the deparsed code, so the syntax error should have become obvious.
... Oh, I see the problem. There are two 'fmt' lines for that clause (and many others), one of which is used when the clause is not present. So there's never a syntax error, because this one never expands other than to empty. AFAICS this defeats the purpose of the 'present' field. I mean, if the clause is never to deparse, then why have it there in the first place? If we want to have it, then it has to be correct. I think we should design the code to avoid the repetition, because that has an inherent risk of typo bugs and such. Maybe we should set forth policy that each 'fmt' string should appear in the source code only once. So instead of this + /* MINVFUNC */ + if (OidIsValid(agg->aggminvtransfn)) + tmp = new_objtree_VA("MINVFUNC=%{type}T", 1, + "type", ObjTypeObject, + new_objtree_for_qualname_id(ProcedureRelationId, + agg->aggminvtransfn)); + else + { + tmp = new_objtree("MINVFUNC==%{type}T"); + append_bool_object(tmp, "present", false); + } we would have something like tmp = new_objtree("MINVFUNC=%{type}T"); if (OidIsValid(agg->aggminvtransfn)) { append_bool_object(tmp, "present", true); append...(tmp, "type", new_objtree_for_qualname_id(ProcedureRelationId, agg->aggminvtransfn)); } else { append_bool_object(tmp, "present", false); } -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "No necesitamos banderas No reconocemos fronteras" (Jorge González)