RelCopyOnWriteVisitor rebuilds a relation when something below it changed and returns Optional.empty() when nothing did, for every relation except four, which throw a bare UnsupportedOperationException() with no message: Expand, ExtensionWrite, NamedDdl and ExtensionDdl.
https://github.com/substrait-io/substrait-java/blob/main/core/src/main/java/io/substrait/relation/RelCopyOnWriteVisitor.java#L275-L306
The gap is already worked around once in main source, which is the clearest statement of what the base class should do. OuterReferenceConverter.RelRewriter overrides visit(NamedDdl) with the comment "The copy-on-write base throws for DDL; a view definition is an independent (top-level) query that may itself contain correlated subqueries, so traverse it rather than throw", and then does exactly that traversal.
Where it is not worked around it fails. SchemaCollector.TableGatherer overrides visit(NamedScan) and visit(NamedWrite) but not visit(NamedDdl), so SubstraitToCalcite.convert(Rel) without a catalog reader — the path that resolves the schema by walking the plan — throws the bare exception on any plan holding a CREATE VIEW, naming neither the relation nor the reason.
The asymmetry inside the class is the part worth closing first: visit(NamedWrite) rebuilds normally while visit(NamedDdl) throws, even though a NamedDdl's view_definition is a Rel in exactly the way a NamedWrite's input is. That matters more now that #1181 gives the DDL relations a declared schema to carry, since a consumer using this visitor to rewrite a plan is the case the schema exists for.
Each of the four can rebuild the way its siblings do — NamedDdl by traversing view_definition as RelRewriter already shows, ExtensionWrite on the model of NamedWrite, Expand through its input and its fields, ExtensionDdl through its own definition. Where one genuinely cannot be rewritten, the throw should at least name the relation it refused.
Found while reviewing #1181, whose new view round-trip test carries a comment explaining that it cannot use assertFullRoundTrip for this reason.
RelCopyOnWriteVisitorrebuilds a relation when something below it changed and returnsOptional.empty()when nothing did, for every relation except four, which throw a bareUnsupportedOperationException()with no message:Expand,ExtensionWrite,NamedDdlandExtensionDdl.https://github.com/substrait-io/substrait-java/blob/main/core/src/main/java/io/substrait/relation/RelCopyOnWriteVisitor.java#L275-L306
The gap is already worked around once in main source, which is the clearest statement of what the base class should do.
OuterReferenceConverter.RelRewriteroverridesvisit(NamedDdl)with the comment "The copy-on-write base throws for DDL; a view definition is an independent (top-level) query that may itself contain correlated subqueries, so traverse it rather than throw", and then does exactly that traversal.Where it is not worked around it fails.
SchemaCollector.TableGathereroverridesvisit(NamedScan)andvisit(NamedWrite)but notvisit(NamedDdl), soSubstraitToCalcite.convert(Rel)without a catalog reader — the path that resolves the schema by walking the plan — throws the bare exception on any plan holding aCREATE VIEW, naming neither the relation nor the reason.The asymmetry inside the class is the part worth closing first:
visit(NamedWrite)rebuilds normally whilevisit(NamedDdl)throws, even though aNamedDdl'sview_definitionis aRelin exactly the way aNamedWrite'sinputis. That matters more now that #1181 gives the DDL relations a declared schema to carry, since a consumer using this visitor to rewrite a plan is the case the schema exists for.Each of the four can rebuild the way its siblings do —
NamedDdlby traversingview_definitionasRelRewriteralready shows,ExtensionWriteon the model ofNamedWrite,Expandthrough its input and its fields,ExtensionDdlthrough its own definition. Where one genuinely cannot be rewritten, the throw should at least name the relation it refused.Found while reviewing #1181, whose new view round-trip test carries a comment explaining that it cannot use
assertFullRoundTripfor this reason.