-
Notifications
You must be signed in to change notification settings - Fork 4.1k
sql: split planNode into planNode and planNodeRun #23522
Copy link
Copy link
Closed
Description
The fix for #23156 introduced in #23373 also introduces a wart:
// batchedPlanNode is an interface that specializes planNode to
// indicate that the local execution behavior operates in batches.
//
// TODO(knz/andrei): nodes that implement this interface, by definition,
// cannot implement planNode's Next() and Values() in the way required
// defined by planNode. This violates the principle that no
// implementer of a derived interface can change any contract of the
// base interfaces - or at least not in ways that can break
// unsuspecting clients of the interface.
// To fix this wart requires splitting planNode into a planNodeBase
// interface, which only supports, say, Close(), and then two
// interfaces that extend planNodeBase; namely serialPlanNode
// providing Next/Values and this new interface batchedPlanNode which
// provides Next/BatchedCount/BatchedValues.
type batchedPlanNode interface { ...This mis-design was left in the code to ensure that code movement in PR #23373 is kept minimal, to let it qualify as a bug fix for the 2.0 release. The proper design with separate interface requires introducing a new interface in the execution code for every node, or a new interface in the logical planning code for every node.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels