IKKBZ Join ordering

[Goblin80]

Finds optimal left-deep tree for an acyclic graph in polynomial time and a necessary prerequisite for search-space linearization.

the cost function has a slight implementation mistake due to a misunderstanding on my behalf, which subsequently has a slight effect on the relation rank. i will fix that ASAP.

[Goblin80]

/cc @joka921

[codecov]

Codecov Report

Attention: Patch coverage is 92.96188% with 24 lines in your changes are missing coverage. Please review.

Project coverage is 88.87%. Comparing base (5e0d8e5) to head (1f5eabf).
Report is 1 commits behind head on master.

Files	Patch %	Lines
src/engine/joinOrdering/QueryGraph.cpp	91.11%	1 Missing and 11 partials ⚠️
src/engine/joinOrdering/IKKBZ.cpp	95.45%	0 Missing and 6 partials ⚠️
src/engine/joinOrdering/CostIKKBZ.cpp	85.71%	0 Missing and 4 partials ⚠️
src/engine/joinOrdering/EdgeInfo.cpp	66.66%	1 Missing ⚠️
src/engine/joinOrdering/GOO.cpp	96.77%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1330      +/-   ##
==========================================
+ Coverage   88.83%   88.87%   +0.03%     
==========================================
  Files         322      329       +7     
  Lines       28548    28897     +349     
  Branches     3149     3220      +71     
==========================================
+ Hits        25361    25682     +321     
- Misses       2046     2051       +5     
- Partials     1141     1164      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[joka921]

Okay, to improve this design I would suggest the following first step:

1. Get the combining of nodes right using the follwing design:

class Relation {
 // ID, selectivity, cardinality
};

// created by the NormalizeStep.
class CombinedRelations {
  std::vector<Relation>; // contained relations.
  double rank() ; // compute the rank.
}

// Called for two consecutive elements in a chain where the rank is out of order
CombinedRelations combine(const CombinedRelations& first, const CombinedRelations& second);


// Run IKKBZ-Normalize on a single chain until it is fully normalized.
void IkkbzNormalize(std::vector<CombinedRelations>& chain);

// Givien the above structure, this is now simple, just merge by rank.
std::vector<CombinedRelations> IkkbzMerge(std::vector<std::vector<CombinedRelations>>);

Maybe start this design in a separate PR, and then star with unit tests for those rather small functions.

Then in a much simpler IKKBZ graph, you can incorporate those structures, but maybe let me know first,
so that we can talk about the next steps.

[joka921]

I think this is much cleaner if

1. The RJoin class is called EdgeInfo.
2. the r is called edges_ and stores std::pair<N, EdgeInfo>`.
3. Use std:unordered_map or better ad_utility::HashMap instead of std::map.

[joka921]

You currently have many copies of the N object. Maybe just store all the relations in one place, and then
only pass around references or pointers (but that can be left open for a later optimization).

[Goblin80]

what the full type signature of edges_?

[joka921]

Okay, to improve this design I would suggest the following first step:

1. Get the combining of nodes right using the follwing design:

class Relation {
 // ID, selectivity, cardinality
};

// created by the NormalizeStep.
class CombinedRelations {
  std::vector<Relation>; // contained relations.
  double rank() ; // compute the rank.
}

// Called for two consecutive elements in a chain where the rank is out of order
CombinedRelations combine(const CombinedRelations& first, const CombinedRelations& second);


// Run IKKBZ-Normalize on a single chain until it is fully normalized.
void IkkbzNormalize(std::vector<CombinedRelations>& chain);

// Givien the above structure, this is now simple, just merge by rank.
std::vector<CombinedRelations> IkkbzMerge(std::vector<std::vector<CombinedRelations>>);

Maybe start this design in a separate PR, and then star with unit tests for those rather small functions.

Then in a much simpler IKKBZ graph, you can incorporate those structures, but maybe let me know first,
so that we can talk about the next steps.

[Goblin80]

I have moved the "merge" out of the query graph. yet I am a little puzzled by this CombinedRelations.

[joka921]

Okay, a combined relation
is a list of relations which have been combined by IKKBZ, because their ranks were out of ORDER
(they are combined into a single node when forming the chains that are ordered by rank).
Does that answer my question?.

So it is for example the two relations "first R3 and then R5" combined into a single graph node which has one rank which can be computed from the cardinalities and selectivities of R3 and R5.
There's examples for this operation in the lecture and exercise slides, so you can use those as unit tests.

[joka921]

I think this is the thread that is worth pursuing for now so we can build up a correct and maintainable implementation of IKKBZ.

[Goblin80]

my argument for keeping the relation as a pure data class is that is way more convenient to pass it around as const& all over that place without mutating it while doing the all the book-keeping necessary to calculate the rank on the QueryGraph.

[sonarcloud]

Quality Gate passed

Issues
63 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud