Skip to content

Commit

Permalink
[fix](catalog) wrong required slot info causing BE crash (apache#21598)
Browse files Browse the repository at this point in the history
For file scan node, this is a special field `requiredSlot`, this field is set depends on the `isMaterialized` info of slot.
But `isMaterialized` info can be changed during the plan process, so we must update the `requiredSlot`
in `finalize` phase of scan node, otherwise, it may causing BE crash due to mismatching slot info.
  • Loading branch information
morningman authored Jul 7, 2023
1 parent 02149ff commit 0b7b5dc
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,8 @@ private FileCacheValue getFileCache(String location, InputFormat<?, ?> inputForm
throw e;
}
}
result.setPartitionValues(partitionValues);
// Must copy the partitionValues to avoid concurrent modification of key and value
result.setPartitionValues(Lists.newArrayList(partitionValues));
return result;
}

Expand Down Expand Up @@ -924,7 +925,7 @@ public boolean equals(Object obj) {
return dummyKey.equals(((FileCacheKey) obj).dummyKey);
}
return location.equals(((FileCacheKey) obj).location)
&& partitionValues.equals(((FileCacheKey) obj).partitionValues);
&& Objects.equals(partitionValues, ((FileCacheKey) obj).partitionValues);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@

/**
* FileQueryScanNode for querying the file access type of catalog, now only support
* hive,hudi, iceberg and TVF.
* hive, hudi, iceberg and TVF.
*/
public abstract class FileQueryScanNode extends FileScanNode {
private static final Logger LOG = LogManager.getLogger(FileQueryScanNode.class);
Expand Down Expand Up @@ -163,6 +163,10 @@ protected void initSchemaParams() throws UserException {
@Override
public void updateRequiredSlots(PlanTranslatorContext planTranslatorContext,
Set<SlotId> requiredByProjectSlotIdSet) throws UserException {
updateRequiredSlots();
}

private void updateRequiredSlots() throws UserException {
params.unsetRequiredSlots();
for (SlotDescriptor slot : desc.getSlots()) {
if (!slot.isMaterialized()) {
Expand Down Expand Up @@ -196,6 +200,7 @@ public void finalizeForNereids() throws UserException {
// Create scan range locations and the statistics.
protected void doFinalize() throws UserException {
createScanRangeLocations();
updateRequiredSlots();
}

private void setColumnPositionMapping()
Expand Down Expand Up @@ -415,3 +420,4 @@ protected static Optional<TFileType> getTFileType(String location) {
return Optional.empty();
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,6 @@ moccasin steel bisque cornsilk lace
-- !q25 --
Z6n2t4XA2n7CXTECJ,PE,iBbsCh0RE1Dd2A,z48

-- !pr21598 --
5

Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ suite("test_external_catalog_hive", "p2") {
sql """ use tpch_1000_orc; """
q03()

// test #21598
qt_pr21598 """select count(*) from( (SELECT r_regionkey AS key1, r_name AS name, pday AS pday FROM (SELECT r_regionkey, r_name, replace(r_comment, ' ', 'aaaa') AS pday FROM ${catalog_name}.tpch_1000_parquet.region) t2))x;"""

// test remember last used database after switch / rename catalog
sql """switch ${catalog_name};"""
Expand Down

0 comments on commit 0b7b5dc

Please sign in to comment.