Skip to content

Commit

Permalink
Fix for TPC-H Q11 (#254)
Browse files Browse the repository at this point in the history
Test failed with `test panicked: Column index 2 out of bounds 2 + 0`

The 0 schema len was due to this part of the plan:
```
            └── LogicalProjection
                ├── exprs:Cast
                │   ├── cast_to: Decimal128(38, 15)
                │   ├── child:Mul
                │   │   ├── Cast { cast_to: Float64, child: #0 }
                │   │   └── 0.0001(float)
```

Turns out, `BinOp` was being considered to have an empty schema length. 

Let me know if this fix does not take into account some case.

---------

Signed-off-by: Alex Chi <iskyzh@gmail.com>
Co-authored-by: Alex Chi <iskyzh@gmail.com>
  • Loading branch information
jurplel and skyzh authored Dec 6, 2024
1 parent 3c81da9 commit 228e7ba
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 47 deletions.
25 changes: 14 additions & 11 deletions optd-datafusion-repr/src/properties/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,19 @@ impl SchemaPropertyBuilder {
Schema { fields }
}
DfPredType::LogOp(_) => Schema {
fields: vec![Field::placeholder(); children.len()],
fields: vec![Field {
typ: ConstantType::Bool,
..Field::placeholder()
}],
},
DfPredType::BinOp(_) => Schema {
fields: vec![Field::placeholder()],
},

DfPredType::Cast => Schema {
fields: children[0]
.fields
.iter()
.map(|field| Field {
typ: children[1].fields[0].typ,
..field.clone()
})
.collect(),
fields: vec![Field {
typ: children[1].fields[0].typ,
..children[0].fields[0].clone()
}],
},
DfPredType::DataType(data_type) => Schema {
fields: vec![Field {
Expand All @@ -144,7 +145,9 @@ impl SchemaPropertyBuilder {
// The real type should be the column type.
fields: vec![Field::placeholder()],
},
_ => Schema { fields: vec![] },
_ => Schema {
fields: vec![Field::placeholder()], // all predicates produce at least one column
},
}
}
}
Expand Down
File renamed without changes.
71 changes: 36 additions & 35 deletions optd-sqlplannertest/tests/tpch/tpch-11-15.planner.sql
Original file line number Diff line number Diff line change
Expand Up @@ -195,45 +195,46 @@ LogicalSort
PhysicalSort
├── exprs:SortOrder { order: Desc }
│ └── #1
└── PhysicalNestedLoopJoin
├── join_type: Inner
├── cond:Gt
│ ├── Cast { cast_to: Decimal128(38, 15), child: #1 }
│ └── #0
├── PhysicalAgg
│ ├── aggrs:Agg(Sum)
│ │ └── Mul
│ │ ├── #2
│ │ └── Cast { cast_to: Decimal128(10, 0), child: #1 }
│ ├── groups: [ #0 ]
│ └── PhysicalProjection { exprs: [ #11, #13, #14 ] }
│ └── PhysicalHashJoin { join_type: Inner, left_keys: [ #4 ], right_keys: [ #1 ] }
│ ├── PhysicalHashJoin { join_type: Inner, left_keys: [ #0 ], right_keys: [ #3 ] }
│ │ ├── PhysicalFilter
│ │ │ ├── cond:Eq
│ │ │ │ ├── #1
│ │ │ │ └── "CHINA"
│ │ │ └── PhysicalScan { table: nation }
│ │ └── PhysicalScan { table: supplier }
│ └── PhysicalScan { table: partsupp }
└── PhysicalProjection
├── exprs:Cast
│ ├── cast_to: Decimal128(38, 15)
│ ├── child:Mul
│ │ ├── Cast { cast_to: Float64, child: #0 }
│ │ └── 0.0001(float)
└── PhysicalProjection { exprs: [ #1, #2 ] }
└── PhysicalNestedLoopJoin
├── join_type: Inner
├── cond:Gt
│ ├── Cast { cast_to: Decimal128(38, 15), child: #2 }
│ └── #0
├── PhysicalProjection
│ ├── exprs:Cast
│ │ ├── cast_to: Decimal128(38, 15)
│ │ ├── child:Mul
│ │ │ ├── Cast { cast_to: Float64, child: #0 }
│ │ │ └── 0.0001(float)
│ └── PhysicalAgg
│ ├── aggrs:Agg(Sum)
│ │ └── Mul
│ │ ├── #1
│ │ └── Cast { cast_to: Decimal128(10, 0), child: #0 }
│ ├── groups: []
│ └── PhysicalProjection { exprs: [ #13, #14 ] }
│ └── PhysicalHashJoin { join_type: Inner, left_keys: [ #4 ], right_keys: [ #1 ] }
│ ├── PhysicalHashJoin { join_type: Inner, left_keys: [ #0 ], right_keys: [ #3 ] }
│ │ ├── PhysicalFilter
│ │ │ ├── cond:Eq
│ │ │ │ ├── #1
│ │ │ │ └── "CHINA"
│ │ │ └── PhysicalScan { table: nation }
│ │ └── PhysicalScan { table: supplier }
│ └── PhysicalScan { table: partsupp }
└── PhysicalAgg
├── aggrs:Agg(Sum)
│ └── Mul
│ ├── #1
│ └── Cast { cast_to: Decimal128(10, 0), child: #0 }
├── groups: []
└── PhysicalProjection { exprs: [ #0, #1 ] }
└── PhysicalHashJoin { join_type: Inner, left_keys: [ #2 ], right_keys: [ #0 ] }
├── PhysicalProjection { exprs: [ #1, #2, #4 ] }
│ └── PhysicalHashJoin { join_type: Inner, left_keys: [ #0 ], right_keys: [ #0 ] }
│ ├── PhysicalProjection { exprs: [ #1, #2, #3 ] }
│ ├── #2
│ └── Cast { cast_to: Decimal128(10, 0), child: #1 }
├── groups: [ #0 ]
└── PhysicalProjection { exprs: [ #0, #1, #2 ] }
└── PhysicalHashJoin { join_type: Inner, left_keys: [ #3 ], right_keys: [ #0 ] }
├── PhysicalProjection { exprs: [ #0, #2, #3, #5 ] }
│ └── PhysicalHashJoin { join_type: Inner, left_keys: [ #1 ], right_keys: [ #0 ] }
│ ├── PhysicalProjection { exprs: [ #0, #1, #2, #3 ] }
│ │ └── PhysicalScan { table: partsupp }
│ └── PhysicalProjection { exprs: [ #0, #3 ] }
│ └── PhysicalScan { table: supplier }
Expand Down
2 changes: 1 addition & 1 deletion optd-sqlplannertest/tests/tpch/tpch-16-20.planner.sql
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ PhysicalProjection
├── cond:And
│ ├── Eq
│ │ ├── #0
│ │ └── #13
│ │ └── #26
│ └── Lt
│ ├── Cast { cast_to: Decimal128(30, 15), child: #13 }
│ └── #25
Expand Down

0 comments on commit 228e7ba

Please sign in to comment.