wombatu-kun commented on code in PR #19863:
URL: https://github.com/apache/hudi/pull/19863#discussion_r3953767458
##########
hudi-trino/src/main/java/io/trino/plugin/hudi/HudiSplitManager.java:
##########
@@ -82,6 +83,12 @@ public ConnectorSplitSource getSplits(
Constraint constraint)
{
HudiTableHandle hudiTableHandle = (HudiTableHandle) tableHandle;
+ // The planner turns a none() domain into an empty ValuesNode before
applyFilter, so a none() predicate
+ // should never reach here; guard anyway, as computePartitionKeyFilter
below throws on a none() domain.
Review Comment:
computePartitionKeyFilter is only ever passed getPartitionPredicates, so it
does not justify the getRegularPredicates arm of this condition - a none()
regular predicate throws nothing there; it only empties
TupleDomainUtils.getReferencedColumns, so every canApply declines and nothing
gets pruned. Worth saying what each arm actually prevents, since they are not
the same thing.
##########
hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiSplitSource.java:
##########
@@ -68,6 +77,29 @@ public void testNoneDynamicFilterTerminatesSource()
assertThat(splitSource.isFinished()).isTrue();
}
+ @Test
+ public void testNonePredicateYieldsEmptySplitSource()
+ {
+ // A none() predicate must short-circuit before the metastore is
consulted: the split manager would
+ // otherwise hand it to computePartitionKeyFilter, which rejects a
none() domain
Review Comment:
createTableHandle passes an empty partitionColumns list, so getPartitions
takes its NON_PARTITION early return and computePartitionKeyFilter is never
reached here. Point the comment at what the guard actually stops in this shape:
splits emitted for a predicate no row can match, which
HiveHudiPartitionInfo.doesMatchPredicates waves through for NON_PARTITION.
##########
hudi-trino/src/main/java/io/trino/plugin/hudi/HudiPredicates.java:
##########
@@ -29,6 +29,10 @@ public class HudiPredicates
public static HudiPredicates from(TupleDomain<ColumnHandle> predicate)
{
+ if (predicate.isNone()) {
+ return new HudiPredicates(TupleDomain.none(), TupleDomain.none());
Review Comment:
This makes applyFilter hand back a none() remaining filter, and
PushPredicateIntoTableScan feeds that straight into computeEnforced, which
opens with checkArgument(!unenforced.isNone()) - so on the input the Impact
section describes, the planner would throw before getSplits and the new guard
would never run. IcebergMetadata and DeltaLakeMetadata both return
TupleDomain.all() as the remaining filter in that case; the same here would
make the stated behavior real.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
