Grammar for plan constraints

[aaneja]

This RFC describes the grammar we would use for specifying plan constraints

[aditi-pandit]

We will have an option for constraint within cte also I presume. I won't push on it, but it would be good to add that example as well.

[aditi-pandit]

Thanks @aaneja

[aaneja]

With #3fcc611 I've update the grammar to make the join specification more explicit.

This disallows specifying grouped relations like (A B C) . Instead we need to specify the full join order like ((A B) C). This will make specifying ambiguous group relations like (A LOJ B ROJ C) impossible

At a later stage, we can enhance the grammar so that inner-join-groups like (A B C) can also be specified - this would be useful to constrain the optimizer to reorder the nodes in it separately from the rest of the join graph

[jaystarshot]

Why can't we just use replicated or paritioned syntax? Imo the brackets and join ((a c [R]) b [P]) might get complicated quickly

[aaneja]

Can you elaborate on what this would look like for the cited example ?

[jaystarshot]

Like dremio or databricks.

Current way also is fine but suggestion is to use explicit names for broadcast/partition etc since the goal of this is to allow users to explictly set the types

[aaneja]

I'm not opposed to this; my only gripe with it is that it that the plan constraint string can get quite verbose, e.g for a 4-table join order - join (d ((a c [REPLICATED]) b [PARTITIONED]) [PARTITIONED] )

[rschlussel]

Do we need this fine grained kind of control. I like the simplicity of just specifying the partitioning of the table, and every other example I found seems to take that approach.

1. Spark: BROADCAST(T1)
2. Oracle: PQ_DISTRIBUTE(s BROADCAST, NONE) (which I also find very clunky).
3. Vertica: you specify inline distrib(L,B)
4. Dremio: mark BROADCAST inline

You can specify to use syntactic join order if you want to control the join ordering in a more complex way.

I find the existing syntax complex and hard to use.

[aaneja]

DB2 supports 'join requests' similar to these join hints. It has variants that specify the join method too
Using a join hint allow us to :

1. Specify partial join order - you don't need to force syntactic join on the rest of the query to force a specific join shape for part of the query tree
2. Hardcode join orders for tools that generate SQL and cannot guarantee syntactic SQL generation

[rschlussel]

Got it. I see the appeal of specifying a partial join order for auto generated queries, but also would like it to be easy to specify join hints for the common case where people just want to mark some table for broadcast or similar. I wonder if there's an alternative approach we can use to achieve both of these goals

[aaneja]

We could add Spark style independent hints -

1. BROADCAST(T1) - Always broadcast T1 in chosen as the build side of a join
2. BROADCAST(a b) - If the optimizer chooses a logical join of a and b`, and this is a sub-tree of another join graph, use BROADCAST for the join distribution

These will be complementary to the join-order syntax. So the below join hints are equivalent but provide flexibility to the user -

BROADCAST(a b)  join(c (ab))
join (c (a b)[R])

Will think of more examples and incorporate them

[kaikalur]

A high level comment - plan level hints make them not accessible to most users. We should keep them more high level like:

BROADCAST
MAPJOIN
HIGHDISTINCT
LOWDISTINCT 
RANDOM_SHUFFLE
FORCE_SHUFFLE
SHARDING
PAYLOAD
SALTING
...

that tell the planner something about the data shape. Planner is a not concept that most users understand. But some of the others we can easily explain to users. Also things fragments can change based on other options. So not sure it's good to use those.

[rschlussel]

It would helpful to describe the syntax of other systems and how this one compares. like if we're doing something different from everyone else we need a very clear reason why.

[aaneja]

I can add a summary section describing the hint syntax, but IMO we can be opinionated about the syntax and make the right tradeoffs for what works best for us.
Happy to discuss this more on a case-by-case basis

[rschlussel]

I agree we can be opinionated and choose what works best for us, but we should still learn from what others do and where we vary, we should understand why we are going a different route/ are consciously choosing it. Being more similar to other systems people may have encountered can also make it more user friendly.

[rschlussel]

just call this PARTITIONED. JOIN_DIST_PARTITIONED is clunky to write and to remember

[aaneja]

I think there's some confusion, the user will use [P] and [R] to specify the constraint specification (see L53-54 for lexer rules), not JOIN_DIST_PARTITIONED or JOIN_DIST_REPLICATED

[rschlussel]

ah, thanks. missed this. Still would recommend using broadcast/ B vs. R because that's more consistent with the user facing language we use elsewhere (e.g. session property join_distribution_type=BROADCAST).

[rschlussel]

I would call this feature Plan Hints rather than Plan Constraints. The optimizer may not follow the hints (though we should issue a warning if we don't)

[aaneja]

I would call this feature Plan Hints rather than Plan Constraints.

Sure, will rename
Agreed on providing a warning if some/all of the plan hint is not applied

[rschlussel]

why is this needed?

[aaneja]

Had mentioned this in an earlier comment, we plan to support join hints for outer joins as well. This provides support for the same. I am OK removing it for now, since v1 will not have support for using this hint

[rschlussel]

but the type of join is determined by the query, so even if we support join hints, we shouldn't need to specify if it's an inner or outer join.

[aaneja]

If/when we support outer join reordering, we would need these to specify hints for outer-join reordering. Of course, if the planner never evaluates these hint choices, these hints are ignored and there is zero chance of a correctness issue w.r.t what type of join to choose

[rschlussel]

Do we need this fine grained kind of control. I like the simplicity of just specifying the partitioning of the table, and every other example I found seems to take that approach.

1. Spark: BROADCAST(T1)
2. Oracle: PQ_DISTRIBUTE(s BROADCAST, NONE) (which I also find very clunky).
3. Vertica: you specify inline distrib(L,B)
4. Dremio: mark BROADCAST inline

You can specify to use syntactic join order if you want to control the join ordering in a more complex way.

I find the existing syntax complex and hard to use.

[rschlussel]

I would rather not expose this kind of control. Better to specify what you want to have happen with this table, and otherwise leave it be. Seems pretty risky to encode cardinality estimates in the query text.

[aaneja]

Can you elaborate on the risk ? I see this card estimate as a simple way to fixup estimation errors for when -

1. Presto doesn't have stats, but the user is aware of maxcard for a base table
2. Presto is unable to figure out the right stats because it has a missing stats rule (e.g see the WITH CTE example at L144 below)
3. Provides a good debugging aid for devs to quickly test what-if scenarios wrt to cardinality estimates

[rschlussel]

The risk I see is that if you are specifying a cardinality of x, you are probably doing it because you want the optimizer to do some particular thing about it. But you don't really know what the optimizer will do with that information, and it could do one thing for a while, and then in a new release there's an optimizer change and it does something else. Because you aren't directly controlling what happens when you specify the cardinality, it's hard to say how it might affect the query, and could be hard to debug if the performance degrades (vs. if you e.g. specify broadcast join and your data gets bigger, it's very clear what happens)

It can also get out of sync with the data (there is always a risk with hand tuning that the optimization will no longer be relevan or will perform worse as the data changes, but specifying a specific cardinality estimate can have more varied and unknown effects).

[aaneja]

I agree that this is a powerful knob to give to the users, one that we can document as such (with caveats)

To aid debugging, we can add CardEstimateBasedSourceInfo similar to how we have a HistoryBasedSourceInfo that will make it clear to the users how the cardinality was arrived at in EXPLAIN/EXPLAIN ANALYZE (and event listener etc.)

Additional safeguards like warnings & metrics can be incorporated too if the stats estimate differ widely from actual runtime observed cardinality