finalizeDt on both sides of the join makes redundant enumeration

[pashandor789]

Example

auto logicalPlan =
      lp::PlanBuilder(context)
          .tableScan("nation")
          .limit(0, 10)
          .orderBy({"n_nationkey"})
          .as("n1")
          .join(
              lp::PlanBuilder(context)
                  .tableScan("region")
                  .limit(0, 5)
                  .orderBy({"r_regionkey"})
                  .as("r1"),
              "n1.n_regionkey = r1.r_regionkey",
              lp::JoinType::kInner)
          .build();

This queries cause a fail below.

E20251021 17:04:08.954496 902249 Exceptions.h:53] Line: /home/ivanovp/projects/verax/axiom/optimizer/Optimization.cpp:1297, Function:crossJoin, Expression:  No cross joins, Source: RUNTIME, ErrorCode: NOT_IMPLEMENTED
unknown file: Failure
C++ exception with description "Exception: VeloxRuntimeError
Error Source: RUNTIME
Error Code: NOT_IMPLEMENTED
Reason: No cross joins

Why is it happening?

translateJoin calls makeQueryGraph(*joinLeft) / makeQueryGraph(*joinRight)

consider we've dt1 when we are entering the translateJoin.
on the left makeQueryGraph we have limit + sort as in example, there's a code

case lp::NodeKind::kSort:
      // Multiple orderBys are allowed before a limit. Last one wins. Previous
      // are dropped. If arrives after limit, then starts a new DT.

      makeQueryGraph(*node.onlyInput(), allowedInDt);

      if (currentDt_->hasLimit()) {
        finalizeDt(*node.onlyInput());
      }

      return addOrderBy(*node.asUnchecked<lp::SortNode>());

so we makeQueryGraph for the input and now currentDt (let it be dt1) has a state:

dt1.tables = [table1]

then we call finalizeDt (code is below), which creates a new one and then put the previous dt into tables of the current
so now the state will be:

dt1.tables = [table1]
dt2.tables = [dt1]

void ToGraph::finalizeDt(
    const lp::LogicalPlanNode& node,
    DerivedTableP outerDt) {
  DerivedTableP dt = currentDt_;
  setDtUsedOutput(dt, node);

  currentDt_ = outerDt != nullptr ? outerDt : newDt();
  currentDt_->addTable(dt);

  dt->makeInitialPlan();
}

so after leaving the left makeQueryGraph, currentDt = dt2
and here we go to the right part which do the same
we call makeQueryGraph and then finalizeDt for sort case.

makeQueryGraph will come across table2 and we will put it into our dt2 so the state is

dt1.tables = [table1]
dt2.tables = [dt1, table2]

and then we call finalizeDt which starts enumeration in the makeInitialPlan
dt2.tables = [dt1, table2] will trigger cross join to be enumerated .

[mbasmanova]

It seems like the problem is that we haven't imported join edges into dt2.

[MBkkt]

@mbasmanova

It seems like the problem is that we haven't imported join edges into dt2.

I'm not sure it's possible because we need join edge between our own DT and other DT.

Now "logical node join to join edge" is asymmetric (for an example here left part is separate dt but right part is not).
I think it's not right and better to make it symmetric, it will simplify things and at the same time we will consider better plans.

[MBkkt]

@mbasmanova I think this issue is related to this fix

580d0ad#diff-392a852edbb7c2f123037c6696e82f9299a880665b44ba79bbc68a5caeea43a0L1538

before this commit it was

1. translateJoin
2. make(left, allow(join))
3. inside make: if (!contains(limit)) wrapInDt

So in old code Pavel test probably will work.
In new code part about contains is missed

I made fix that doesn't revert new behavior but accounts different (compared to other nodes) join behavior better

[mbasmanova]

@MBkkt @pashandor789 It seems like a fix can be quite simple: #553

[pashandor789]

@mbasmanova yes it fixes that particular case.
But the problem will still occure when we call finalizeDt on both sides of the join
It still can be possible to break it i think

[mbasmanova]

@pashandor789 Do you have an example that doesn't work?

[pashandor789]

well didn't notice that it put allowedDt = join only in translateJoin
so in terms of this it now works yes