chovy-3012 commented on PR #13088:
URL: https://github.com/apache/seatunnel/pull/13088#issuecomment-6709634717
Thanks @DanielLeens for the detailed review! All five issues are addressed
in `c74e444`.
**Issue 1 — planning executed a real, uncapped query (High)**
Schema discovery in query mode now always goes through a zero-cost dry run,
so planning never executes or bills the query:
```text
createSource
└─ resolveFromQuery
├─ discoverSchemaViaDryRun (always)
│ newQueryJobConfigBuilder(query, config) // shared with the
reader:
│ .setDryRun(true).build() // useQueryCache +
maximumBytesBilled + defaultDataset
│ location → JobId.setLocation()
│ bigQuery.create(JobInfo) // jobs.insert, no
execution
│ return QueryStatistics.getSchema()
├─ schema == null (e.g. emulator) → WARN
│ └─ inferSchemaFromQuery // fallback only:
maxResults=1, same shared config
└─ schema still null/empty → fail fast: "Query returned no schema"
```
The `dry_run` option now affects runtime only: with `dry_run = true` the
reader still performs a dry run, logs the estimate and emits no rows.
Regression tests assert the submitted job config (`dryRun()` true,
`maximumBytesBilled` set, `query()` never called). Verified on the BigQuery
emulator: both a dry-run job and a normal job plan via dry run (0 and 3 rows
emitted), and an invalid query fails at planning with a clear error instead of
at runtime.
**Issue 2 — idle readers never terminate (High)**
`run()` / `registerReader()` / `addSplitsBack()` now signal no-more-splits
to every idle, late-registered and restored reader via `splitsDiscovered` /
`assignedReader` tracking. Covered by tests for two readers, late registration
and empty restored state.
**Issue 3 — deprecated requirePartitionFilter location (Medium)**
`checkRequirePartitionFilter` reads the top-level
`table.getRequirePartitionFilter()` first, keeping the legacy
`TimePartitioning` fallback. Tested for range partitioning (top-level flag
only) and the legacy nested field.
**Issue 4 — Spotless failure on BigqueryIT (Medium)**
The wrapped `executeJob` assignment is merged to one line.
**Issue 5 — missing contract docs (Medium)**
Added Javadoc for the reader lifecycle (checkpoint locking, whole-query
replay), `snapshotState`, conversion contracts and enumerator assignment.
Also hardened while self-reviewing: `dry_run` option description aligned
with the runtime-only semantics, fail-fast when both discovery stages return no
schema, a shared query-config builder between factory and reader, and a
null-estimate log guard for emulator dry runs.
--
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]
