apache / apache/iceberg-go

table: PlanFiles skips rowFilter validation on empty planning paths

Open
#1,119 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
463
Forks
232
Avg merge
2d 12h
Merged PRs (30d)
118

Description

`Scan.PlanFiles()` validates malformed `rowFilter` expressions inconsistently.

If planning exits before any evaluator is constructed, invalid filters can be accepted without error.

### Code path

```text
PlanFiles
-> fetchPartitionSpecFilteredManifests
-> if scan.Snapshot() == nil { return nil, nil }
-> manifestEvaluators := newKeyDefaultMapWrapErr(scan.buildManifestEvaluator)
-> buildManifestEvaluator
-> partitionFilters.Get(specID)
-> buildPartitionProjection
-> newInclusiveProjection(...).Project(rowFilter)
-> BindExpr(...)
-> if len(manifestList) == 0 { return nil, nil }
-> collectManifestEntries
-> newInclusiveMetricsEvaluator(..., scan.rowFilter, ...)
-> BindExpr(...)
-> partitionEvaluators := newKeyDefaultMapWrapErr(scan.buildPartitionEvaluator)
-> buildPartitionEvaluator
-> ExpressionEvaluator(... partitionFilters.Get(specID) ...)
```

Observed empty-planning cases:

- no current snapshot: `fetchPartitionSpecFilteredManifests()` returns `nil, nil`
- current snapshot with an empty manifest list: `PlanFiles()` returns early on `len(manifestList) == 0`

In both cases, the usual filter-binding paths are never reached.

Relevant code:

- `table/scanner.go:76`
- `table/scanner.go:286`
- `table/scanner.go:300`
- `table/scanner.go:314`
- `table/scanner.go:466`
- `table/scanner.go:500`
- `table/scanner.go:517`
- `table/scanner.go:567`
- `table/scanner.go:675`
- `table/evaluators.go:41`
- `table/evaluators.go:573`
- `table/evaluators.go:676`

### Current behavior

For a malformed scan filter:

- on an empty planning path, `PlanFiles()` can return no tasks and no error
- on a non-empty planning path, validation is reached once evaluator creation runs
- because the lazy evaluator caches use `newKeyDefaultMapWrapErr(...)`, those binding errors may currently surface via `panic` instead of a normal returned error

### Expected behavior

`PlanFiles()` should reject malformed `rowFilter` expressions regardless of whether planning is empty.

In practice, validation likely needs to happen before the early return for empty planning paths.

### Scope

This looks like a validation-timing issue in `PlanFiles()`, not a case-sensitivity issue.

`ReadTasks()` appears narrower here because it binds `scan.rowFilter` eagerly before reading.

Contributor guide

Open the contributing guide

Research direction

Start in table/scanner.go at PlanFiles and trace the listed early returns, then inspect evaluator binding in table/evaluators.go. Confirm the fix when malformed rowFilter expressions return a normal error on both no-snapshot and empty-manifest paths, rather than being silently accepted or surfacing via panic.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
data-engineering, databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.