Add TableScanBuilder in PlanBuilder

[majetideepak]

Description

We currently have 4 APIs for TableScan in PlanBuilder
This was likely to enable optional arguments. The connectorId is still hard-coded.

We should instead have a TableScanBuilder on the lines of HiveConnectorSplitBuilder
The API will look like
tableScanBuilder().outputType(rowTypePtr).subfieldFilters({"c1 = true"}).remainingFilter{"c0 < 100"}.build();
This is longer but less confusing and simplifies the API. It is also more descriptive compared to tableScan(rowTypePtr, {"c1 = true"}, "c0 < 100").

[majetideepak]

@mbasmanova any thoughts?

[mbasmanova]

At a high level, I like this idea, but can you show a working example? How will you add nodes on top of TableScan node built using tableScanBuilder?

[majetideepak]

It will look like

auto tableScan = tableScanBuilder()
          .name("hive-table")
          .outputType(rowTypePtr)
          .subfieldFilters({"c1 = true"})
          .remainingFilter{"c0 < 100"}
          .build();
plan = PlanBuilder().add(tableScan).planNode();

[mbasmanova]

I see, what about plans with multiple table scans (joins)? Would something like below be possible / makes sense?

plan = PlanBuilder(planNodeIdGenerator)
   .startTableScan()
          .table("hive-table")
          .outputType(rowTypePtr)
          .subfieldFilters({"c1 = true"})
          .remainingFilter("c0 < 100")
   .endTableScan()
   .planNode();

[majetideepak]

tableScanBuilder() will return a PlanNodePtr of type TableScanNode. add() API in PlanBuilder will simply assign that to planNode_ field and return the reference. tableScanBuilder() is an independent class separate from PlanBuilder().
We can inline the builder. The problem with your proposal is that the API such as table(), outputType() will be part of the PlanBuilder class and we will have to add checks to ensure table() is invoked in the right context. That will complicate the API further.

Multi scans/joins will look like below. The following is an extract from TpchQueryBuilder

  auto customers = PlanBuilder(planNodeIdGenerator, pool_.get())
                       .add(TableScanBuilder()
                           .table(kCustomer)
                           .outputType(customerSelectedRowType)
                           .columnAliases(customerFileColumns)
                           .subfieldFilters({customerFilter})
                           .build())
                       .capturePlanNodeId(customerPlanNodeId)
                       .planNode();

  auto custkeyJoinNode =
      PlanBuilder(planNodeIdGenerator, pool_.get())
           .add(TableScanBuilder()
              .table(kOrders)
              .outputType(ordersSelectedRowType)
              .columnAliases(ordersFileColumns)
              .subfieldFilters({orderDateFilter})
             .build())
          .capturePlanNodeId(ordersPlanNodeId)
          .hashJoin(
              {"o_custkey"},
              {"c_custkey"},
              customers,
              "",
              {"o_orderdate", "o_shippriority", "o_orderkey"})
          .planNode(); 

[mbasmanova]

@majetideepak Deepak, you need to make sure different TableScan nodes have different node IDs (in the same query plan).

The problem with your proposal is that the API such as table(), outputType() will be part of the PlanBuilder class

Not necessarily.

• PlanBuilder::startTableScan() returns TableScanBuilder&.
• TableScanBuilder::endTableScan() returns PlanBuilder&.

[majetideepak]

you need to make sure different TableScan nodes have different node IDs (in the same query plan)

I see we have nextPlanNodeId() as an argument. I guess it is convenient to have it sequential and not a random Id across plan nodes.
Thanks for clarifying your API. That makes sense!
I will work on that.

[majetideepak]

We could also add a new API setId() to PlanNode and make it work inside add(). But startTableScan() and endTableScan() works.

[mbasmanova]

add a new API setId()

It would be preferable to keep PlanNode classes immutable.

[majetideepak]

Completed!