Implement deep copies for the Operation subclasses.
#1815
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1815 +/- ##
==========================================
+ Coverage 90.12% 90.19% +0.06%
==========================================
Files 399 399
Lines 38220 38399 +179
Branches 4283 4294 +11
==========================================
+ Hits 34446 34634 +188
+ Misses 2483 2470 -13
- Partials 1291 1295 +4 ☔ View full report in Codecov by Sentry. |
RobinTF
force-pushed
the
cloneable-operation
branch
from
150a56b to
3f8f44a
Compare
| auto newJoin = std::make_unique<ExistsJoin>(*this); | ||
| newJoin->left_ = left_->clone(); | ||
| newJoin->right_ = right_->clone(); | ||
| return newJoin; |
There was a problem hiding this comment.
How much does it break if you delete the copy constructor of the Operation base class, such that this pattern doesn't work anymore, and then always explicitly call appropriate constructors?
|
|
||
| // _____________________________________________________________________________ | ||
| std::unique_ptr<Operation> Minus::clone() const { | ||
| auto copy = std::make_unique<Minus>(*this); |
| } | ||
| return true; | ||
| }; | ||
| AD_CORRECTNESS_CHECK(areChildrenDifferent()); |
There was a problem hiding this comment.
Can also include expensive checks for the equality of variableToColumnMaps, sizeEstimates, multiplicities, whatNotElse
There was a problem hiding this comment.
(have a look at the public interface of Operation for inspiration.
There was a problem hiding this comment.
Many of these don't work because their public interface is not const (even though it seems like it could be const for most of them)
There was a problem hiding this comment.
The really last request:
In the unit test matcher of clone,
you can assert that the varToColMaps and Multiplicities are equal, and that (most importantly) the getResult yields equal results.
Otherwise this is ready to merge (so you can rebase all your code that depens on it on this PR.
Conformance check passed ✅No test result changes. |
|
joka921
changed the title
Operation classOperation subclasses.
The deep copies are implemented via an explicit
clonemethods.The reason behind this explicit method is two-fold:
shared_ptrs to make the query planning cheaper, so we need explicit logic to recursively deep clone all the children.clonemethod anyway, becauseOperationis a virtual base class, where we need an explicitclonemethod (the socalledvirtual copy constructoridiom, to create a copy of a pointer to the virtual base class.