Skip to content

Commit 6b7781f

Browse files
committed
address review
1 parent 4f71fdc commit 6b7781f

2 files changed

Lines changed: 83 additions & 84 deletions

File tree

fluss-flink/fluss-flink-common/src/main/java/org/apache/fluss/flink/source/FlinkTableSource.java

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.apache.fluss.config.StatisticsColumnsConfig;
2525
import org.apache.fluss.config.TableConfig;
2626
import org.apache.fluss.flink.FlinkConnectorOptions;
27+
import org.apache.fluss.flink.row.FlinkAsFlussRow;
2728
import org.apache.fluss.flink.source.deserializer.RowDataDeserializationSchema;
2829
import org.apache.fluss.flink.source.lookup.FlinkAsyncLookupFunction;
2930
import org.apache.fluss.flink.source.lookup.FlinkLookupFunction;
@@ -46,7 +47,6 @@
4647
import org.apache.fluss.predicate.Predicate;
4748
import org.apache.fluss.predicate.PredicateBuilder;
4849
import org.apache.fluss.predicate.PredicateVisitor;
49-
import org.apache.fluss.row.GenericRow;
5050
import org.apache.fluss.types.DataTypeChecks;
5151
import org.apache.fluss.types.RowType;
5252

@@ -105,7 +105,6 @@
105105
import static org.apache.fluss.flink.utils.LakeSourceUtils.createLakeSource;
106106
import static org.apache.fluss.flink.utils.PredicateConverter.convertToFlussPredicate;
107107
import static org.apache.fluss.flink.utils.PushdownUtils.ValueConversion.FLINK_INTERNAL_VALUE;
108-
import static org.apache.fluss.flink.utils.PushdownUtils.ValueConversion.FLUSS_INTERNAL_VALUE;
109108
import static org.apache.fluss.flink.utils.PushdownUtils.extractFieldEquals;
110109
import static org.apache.fluss.utils.Preconditions.checkNotNull;
111110

@@ -599,7 +598,7 @@ && hasPrimaryKey()
599598
// if not all primary key fields are in condition, fall through to
600599
// try partition filter pushdown for partitioned PK tables
601600
if (visitedPkFields.equals(primaryKeyTypes.keySet())
602-
&& lookupCoversAllData(filters, primaryKeyTypes)) {
601+
&& lookupCoversAllData(lookupRow)) {
603602
singleRowFilter = lookupRow;
604603
// FLINK-38635: return all filters as remaining for scan vs lookup safety net
605604
return Result.of(acceptedFilters, filters);
@@ -894,34 +893,23 @@ private int[] getKeyRowProjection() {
894893
return projection;
895894
}
896895

897-
private boolean lookupCoversAllData(
898-
List<ResolvedExpression> filters, Map<Integer, LogicalType> primaryKeyTypes) {
896+
private boolean lookupCoversAllData(GenericRowData lookupRow) {
899897
if (!isDataLakeEnabled || !isPartitioned()) {
900898
return true;
901899
}
902-
return PushdownUtils.partitionExists(
903-
tablePath, flussConfig, resolveLookupPartition(filters, primaryKeyTypes));
904-
}
905-
906-
private PartitionSpec resolveLookupPartition(
907-
List<ResolvedExpression> filters, Map<Integer, LogicalType> primaryKeyTypes) {
908-
List<FieldEqual> keyEquals =
909-
extractFieldEquals(
910-
filters,
911-
primaryKeyTypes,
912-
new ArrayList<>(),
913-
new ArrayList<>(),
914-
FLUSS_INTERNAL_VALUE);
915-
int[] keyRowProjection = getKeyRowProjection();
916-
GenericRow keyRow = new GenericRow(primaryKeyIndexes.length);
917-
for (FieldEqual keyEqual : keyEquals) {
918-
keyRow.setField(keyRowProjection[keyEqual.fieldIndex], keyEqual.equalValue);
919-
}
900+
// TODO: drop this gate once FIP-28 lets the lookup path read expired partitions from the
901+
// lake; then always push the single-row lookup down instead of falling back to a scan.
902+
// Partition keys are a subset of the primary key, so the partition resolves from lookupRow.
920903
RowType flussRowType = FlinkConversions.toFlussRowType(tableOutputType);
921-
List<String> partitionKeys = flussRowType.project(partitionKeyIndexes).getFieldNames();
922-
return new PartitionGetter(flussRowType.project(primaryKeyIndexes), partitionKeys)
923-
.getResolvedPartitionSpec(keyRow)
924-
.toPartitionSpec();
904+
PartitionGetter partitionGetter =
905+
new PartitionGetter(
906+
flussRowType.project(primaryKeyIndexes),
907+
flussRowType.project(partitionKeyIndexes).getFieldNames());
908+
PartitionSpec partitionSpec =
909+
partitionGetter
910+
.getResolvedPartitionSpec(new FlinkAsFlussRow(lookupRow))
911+
.toPartitionSpec();
912+
return PushdownUtils.partitionExists(tablePath, flussConfig, partitionSpec);
925913
}
926914

927915
@VisibleForTesting

fluss-lake/fluss-lake-paimon/src/test/java/org/apache/fluss/lake/paimon/flink/FlinkUnionReadPrimaryKeyTableITCase.java

Lines changed: 68 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,63 +1123,74 @@ void testUnionReadPartitionsExistInPaimonButExpiredInFluss() throws Exception {
11231123
void testPointLookupOnExpiredPartitionReadsFromLake() throws Exception {
11241124
JobClient jobClient = buildTieringJob(execEnv);
11251125

1126-
String tableName = "point_lookup_expired_partition_pk_table";
1127-
TablePath tablePath = TablePath.of(DEFAULT_DB, tableName);
1128-
Map<TableBucket, Long> bucketLogEndOffset = new HashMap<>();
1129-
Function<String, List<InternalRow>> rowGenerator =
1130-
(partition) ->
1131-
Arrays.asList(
1132-
row(3, "string", partition), row(30, "another_string", partition));
1133-
long tableId =
1134-
prepareSimplePKTable(
1135-
tablePath, DEFAULT_BUCKET_NUM, true, rowGenerator, bucketLogEndOffset);
1136-
1137-
waitUntilBucketSynced(tablePath, tableId, DEFAULT_BUCKET_NUM, true);
1138-
1139-
Map<Long, String> partitionNameByIds = waitUntilPartitions(tablePath);
1140-
assertThat(partitionNameByIds.size()).isGreaterThanOrEqualTo(2);
1141-
1142-
// stop tiering so the read is served from the lake snapshot
1143-
jobClient.cancel().get();
1144-
1145-
Iterator<String> partitionIterator = partitionNameByIds.values().iterator();
1146-
String expiredPartition = partitionIterator.next();
1147-
String livePartition = partitionIterator.next();
1148-
1149-
admin.dropPartition(
1150-
tablePath,
1151-
new PartitionSpec(Collections.singletonMap("c3", expiredPartition)),
1152-
false)
1153-
.get();
1154-
retry(
1155-
Duration.ofSeconds(60),
1156-
() ->
1157-
assertThat(admin.listPartitionInfos(tablePath).get())
1158-
.noneMatch(p -> expiredPartition.equals(p.getPartitionName())));
1159-
1160-
List<String> expiredResult =
1161-
collectBatchRows(
1162-
batchTEnv
1163-
.executeSql(
1164-
String.format(
1165-
"select * from %s where c1 = 3 and c3 = '%s'",
1166-
tableName, expiredPartition))
1167-
.collect());
1168-
assertThat(expiredResult)
1169-
.as("point query on a lake-only (expired) partition must read from the lake")
1170-
.containsExactly(String.format("+I[3, string, %s]", expiredPartition));
1171-
1172-
List<String> liveResult =
1173-
collectBatchRows(
1174-
batchTEnv
1175-
.executeSql(
1176-
String.format(
1177-
"select * from %s where c1 = 3 and c3 = '%s'",
1178-
tableName, livePartition))
1179-
.collect());
1180-
assertThat(liveResult)
1181-
.as("point query on a live partition must still return its row")
1182-
.containsExactly(String.format("+I[3, string, %s]", livePartition));
1126+
boolean tieringCancelled = false;
1127+
try {
1128+
String tableName = "point_lookup_expired_partition_pk_table";
1129+
TablePath tablePath = TablePath.of(DEFAULT_DB, tableName);
1130+
Map<TableBucket, Long> bucketLogEndOffset = new HashMap<>();
1131+
Function<String, List<InternalRow>> rowGenerator =
1132+
(partition) ->
1133+
Arrays.asList(
1134+
row(3, "string", partition),
1135+
row(30, "another_string", partition));
1136+
long tableId =
1137+
prepareSimplePKTable(
1138+
tablePath, DEFAULT_BUCKET_NUM, true, rowGenerator, bucketLogEndOffset);
1139+
1140+
waitUntilBucketSynced(tablePath, tableId, DEFAULT_BUCKET_NUM, true);
1141+
1142+
Map<Long, String> partitionNameByIds = waitUntilPartitions(tablePath);
1143+
assertThat(partitionNameByIds.size()).isGreaterThanOrEqualTo(2);
1144+
1145+
// stop tiering so the read is served from the lake snapshot; the per-job MiniCluster
1146+
// shuts down with the job, so cancel exactly once
1147+
jobClient.cancel().get();
1148+
tieringCancelled = true;
1149+
1150+
Iterator<String> partitionIterator = partitionNameByIds.values().iterator();
1151+
String expiredPartition = partitionIterator.next();
1152+
String livePartition = partitionIterator.next();
1153+
1154+
admin.dropPartition(
1155+
tablePath,
1156+
new PartitionSpec(Collections.singletonMap("c3", expiredPartition)),
1157+
false)
1158+
.get();
1159+
retry(
1160+
Duration.ofSeconds(60),
1161+
() ->
1162+
assertThat(admin.listPartitionInfos(tablePath).get())
1163+
.noneMatch(p -> expiredPartition.equals(p.getPartitionName())));
1164+
1165+
List<String> expiredResult =
1166+
collectBatchRows(
1167+
batchTEnv
1168+
.executeSql(
1169+
String.format(
1170+
"select * from %s where c1 = 3 and c3 = '%s'",
1171+
tableName, expiredPartition))
1172+
.collect());
1173+
assertThat(expiredResult)
1174+
.as("point query on a lake-only (expired) partition must read from the lake")
1175+
.containsExactly(String.format("+I[3, string, %s]", expiredPartition));
1176+
1177+
List<String> liveResult =
1178+
collectBatchRows(
1179+
batchTEnv
1180+
.executeSql(
1181+
String.format(
1182+
"select * from %s where c1 = 3 and c3 = '%s'",
1183+
tableName, livePartition))
1184+
.collect());
1185+
assertThat(liveResult)
1186+
.as("point query on a live partition must still return its row")
1187+
.containsExactly(String.format("+I[3, string, %s]", livePartition));
1188+
} finally {
1189+
// only cancel here if setup failed before the intended cancel above
1190+
if (!tieringCancelled) {
1191+
jobClient.cancel().get();
1192+
}
1193+
}
11831194
}
11841195

11851196
@Test

0 commit comments

Comments
 (0)