[SERVER-71774] [CQF] Improve type safety and error messages for algebra::transport Created: 01/Dec/22  Updated: 09/Dec/22  Resolved: 09/Dec/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Major - P3
Reporter: David Percy Assignee: Backlog - Query Optimization
Resolution: Won't Fix Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Assigned Teams:
Query Optimization
Participants:

 Description   

algebra::transport takes a class with a transport() and prepare() method, with one overload per case of the PolyValue. Often we only care about a few cases, so we have a catch-all overload that handles any type.

If you make a mistake in the type signature of any of the overloads, you might not get a compile error. It will just be dead code, and algebra::transport will call the fallback overload in that case. (Or in the case of prepare(), algebra::transport will just not call it.)

I wonder if we can improve this by defining a Transport interface. Something like this:

template<typename Result>
class Transport {
public:
    void prepare(ABT&, const ScanNode&) {}
    Result transport(ABT&, const ScanNode&) = 0;
    ...
};

Then an individual transport implementation can opt-in to better type errors like this:

class VariableCollector : Transport<void> {
    void prepare(ABT&, const ScanNode&) override { ... }
    Result transport(ABT&, const ScanNode&) override { ... }
};

The 'override' ensures the method signatures are actually the ones the caller is expecting.



 Comments   
Comment by David Percy [ 01/Dec/22 ]

One problem: in the definition of the base class, would we need to spell out each case separately? Would we need a different base class for each PolyValue?

template<class R>
class ABTTransport {
public:
    R transport(ABT&, const ScanNode&) = 0;
    R transport(ABT&, const IndexScanNode&) = 0;
    ...
};
 
template<class R>
class BoolExprTransport {
public:
    R transport(ABT& const Conjunction&) = 0;
    R transport(ABT& const Disjunction&) = 0;
    ...
};
 
...

This seems like a maintenance problem as we add new cases, and new instantiations of PolyValue.

Or could we make it generic for any PolyValue?

template<class Input, class Result>
class Transport {
public:
    // Generate the overloads based on traits of Input... ?
};

Generated at Thu Feb 08 06:19:55 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.