Skip to content

Commit

Permalink
fix: Fix issue where we ended up with a broken tuple vector due to mi…
Browse files Browse the repository at this point in the history
…x of schema columns when having left joins with no matching rows

fix: Fix issue where outer context got cleared in a outer apply chain ending up with a broken tuple vector that does not match the schema
  • Loading branch information
kuseman committed Aug 13, 2024
1 parent 3e386a8 commit 9af1618
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import java.util.List;
import java.util.function.IntPredicate;

import org.apache.commons.lang3.StringUtils;

import se.kuseman.payloadbuilder.api.catalog.Column;
import se.kuseman.payloadbuilder.api.catalog.Schema;
import se.kuseman.payloadbuilder.api.execution.TupleVector;
Expand Down Expand Up @@ -276,8 +278,15 @@ private boolean columnsEquals(List<Column> existingColumns, List<Column> schemaC

for (int i = 0; i < size; i++)
{
if (!existingColumns.get(i)
.equals(schemaColumns.get(i)))
// NOTE! We only compare column names here and not it's type
// When running a asterisk query we might end up with same column names but different types.
// For example when having a left join with no matching inner rows we use the compile time schema
// for the inner and those will be ANY and if combined with a vector with rows we will have the real runtime
// type and we will end up with a wrong resulting vector.
if (!StringUtils.equalsIgnoreCase(existingColumns.get(i)
.getName(),
schemaColumns.get(i)
.getName()))
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import se.kuseman.payloadbuilder.core.common.DescribableNode;
import se.kuseman.payloadbuilder.core.common.SchemaUtils;
import se.kuseman.payloadbuilder.core.execution.ExecutionContext;
import se.kuseman.payloadbuilder.core.execution.StatementContext;
import se.kuseman.payloadbuilder.core.execution.ValueVectorAdapter;
import se.kuseman.payloadbuilder.core.execution.VectorUtils;
import se.kuseman.payloadbuilder.core.execution.vector.TupleVectorBuilder;
Expand Down Expand Up @@ -248,6 +249,9 @@ public Schema getSchema()
@Override
public TupleIterator execute(IExecutionContext context)
{
// Extract outer tuple vector before executing outer in case it's cleared inside that branch
TupleVector outerTupleVector = ((StatementContext) context.getStatementContext()).getOuterTupleVector();

final TupleIterator outerIt = outer.execute(context);

if (!outerIt.hasNext())
Expand All @@ -262,7 +266,7 @@ public TupleIterator execute(IExecutionContext context)
if (condition == null
&& !outerReferences.isEmpty())
{
return new LoopTupleIterator((ExecutionContext) context, outerIt, nodeData);
return new LoopTupleIterator((ExecutionContext) context, outerIt, outerTupleVector, nodeData);
}
else if (populateAlias != null)
{
Expand Down Expand Up @@ -785,13 +789,12 @@ private class LoopTupleIterator implements TupleIterator
private Schema innerSchema = inner.getSchema();
private boolean innerSchemaAsterisk = SchemaUtils.isAsterisk(innerSchema);

LoopTupleIterator(ExecutionContext context, TupleIterator iterator, LoopNodeData nodeData)
LoopTupleIterator(ExecutionContext context, TupleIterator iterator, TupleVector contextOuterTupleVector, LoopNodeData nodeData)
{
this.context = context;
this.iterator = iterator;
this.nodeData = nodeData;
this.outerTupleVector = new OuterTupleVector(context.getStatementContext()
.getOuterTupleVector());
this.outerTupleVector = new OuterTupleVector(contextOuterTupleVector);
}

@Override
Expand Down
41 changes: 40 additions & 1 deletion payloadbuilder-core/src/test/resources/harnessCases/Joins.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@
["idx1", 1, [ { "wareh_id": 1, "qty": 1 } ]],
["idx2", 2, [ { "wareh_id": 2, "qty": 1 } ]]
]
},
{
"name": "pvalues",
"columns": ["pvalue"],
"rows": [
[ 1 ],
[ 2 ]
]
}
]
}
Expand Down Expand Up @@ -617,6 +625,37 @@
[{ "key": "valueX", "value": 2 }, { "key": "valueY", "value": 2 }]
]
]
},
{
"name": "Nested applys",
"description": [
"Verify that the outer context isn't cleared during nested applys when a non loop join is in between.",
"Regression: Happend in schema full in nested loops"
],
"query": [
"--set @@printplan = true ",
"set @@force_nested_loop = true ",
"",
"select * ",
"from range (1,3) x ",
"outer apply ( ",
" select * ",
" from range(1,3) z ",
" inner join pvalues p ",
" on p.pvalue = z.value ",
" cross apply range(0, p.pvalue) zz ",
") y"
],
"expectedResultSets": [
[
[{ "key": "Value", "value": 1 }, { "key": "Value", "value": 1 }, { "key": "pvalue", "value": 1 }, { "key": "Value", "value": 0 }],
[{ "key": "Value", "value": 1 }, { "key": "Value", "value": 2 }, { "key": "pvalue", "value": 2 }, { "key": "Value", "value": 0 }],
[{ "key": "Value", "value": 1 }, { "key": "Value", "value": 2 }, { "key": "pvalue", "value": 2 }, { "key": "Value", "value": 1 }],
[{ "key": "Value", "value": 2 }, { "key": "Value", "value": 1 }, { "key": "pvalue", "value": 1 }, { "key": "Value", "value": 0 }],
[{ "key": "Value", "value": 2 }, { "key": "Value", "value": 2 }, { "key": "pvalue", "value": 2 }, { "key": "Value", "value": 0 }],
[{ "key": "Value", "value": 2 }, { "key": "Value", "value": 2 }, { "key": "pvalue", "value": 2 }, { "key": "Value", "value": 1 }]
]
]
}
]
}
}

0 comments on commit 9af1618

Please sign in to comment.