-
Notifications
You must be signed in to change notification settings - Fork 164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Are RelRoot
s the "results" of a Plan
?
#612
Comments
For most plans there is only one PlanRel which contains a root. That represents an entire plan. The reason you can have multiple PlanRels I believe is for communication purposes (say a backend trying to provide a portion or multiple portions of a plan to another). |
This sounds like all What about I think it would be a good idea to distinguish between relations returned by the plan and relations that are just temporary. |
I think that it is safe to inline I don't know if it is safe to declare that there is only one
I think we're open to suggestions here. I think part of the reason this has been left out of the website is that the protobuf is essentially just a "container of plans" and the interpretation of the container layout is left up to the user. In other words, the following "example applications" would all be using substrait correctly, even though they have different interpretations about what the plans in the file are:
As a result, it is difficult for Substrait to define what exactly a "RelRoot" is. However, I do think we could provide some basic description of a "plan" concept with the idea that there are "root plans" and "not root plans" so that the first paragraph in this message is formally defined and intermediate libraries are free to optimize / discard "not root plans". |
Thanks a lot for the detailed reply! The various use cases are quite interesting, actually! So I think that there is an agreement that there are some I think that this is compatible with all examples above and independent of what exactly "root" It also sounds like |
Borderline off-topic: I am thinking of an analogy to symbol visibility in C/C++: symbols can be Similarly, the |
Let me add two data points to the discussion:
I think that this shows that the two groups of developers have not understood the specification in the same way. |
Acero was a pretty early adopter of Substrait. A common problem we encountered was receiving plans that were technically invalid but also generally understandable. Since Acero is not a validator and we didn't want to be blocked by producer bugs we went ahead and made the consumer fairly permissive where it seemed reasonably straightforward what the intention was. Another example of this is that most producers do not set the URI of functions (or set it to "/") which Acero tolerates as well. When Acero produces plans it always generates a single |
I have made an attempt at clarifying the behavior in #616 |
I am wondering what the "results" of a plan are, i.e., which of the
PlanRel
s of therelations
field are expected to be "returned". More concretely, which of thePlanRel
s could be optimized away or inlined into the other relations and which of them can't (because they are "results" and thus have to be present)? My guess is that that's the purpose ofRelRoot
(apart from specifying names) but I don't see it specified clearly.No matter what the answer is, I think it'd be a good idea to add this to the specification, both in the
proto
files and the documentation/website (which currently doesn't mentionPlanRel
at all, AFAIK).The text was updated successfully, but these errors were encountered: