hreddygayam commented on issue #15699:
URL: https://github.com/apache/iceberg/issues/15699#issuecomment-6095276840
Picking this back up since the thread stalled on direction. I went through
the Spark 4.1/4.2 code and I think @vbhanuchander-lang's structural read is
correct and decisive: the proposed `VERSION BETWEEN … AND …` SELECT syntax
can't land in Iceberg alone.
Confirming the three blockers against current code:
1. `VERSION AS OF` / `TIMESTAMP AS OF` are Spark's grammar; Iceberg only
implements the catalog time-travel callbacks.
2. The `statement` rule in `IcebergSqlExtensions.g4` has only `ALTER TABLE`
forms — no relation / `FROM`-clause syntax.
3. `IcebergSparkSqlExtensionsParser` is whole-statement and
`isIcebergCommand` only matches `"alter table …"`, so any `SELECT` is delegated
wholesale to Spark's parser, which doesn't know `VERSION BETWEEN`.
So the literal syntax is a Spark change, and that decision belongs upstream
in Spark, not here.
## Proposal: a table-valued function, fully inside Iceberg
The scan side already exists — `SparkReadOptions.START_SNAPSHOT_ID` /
`END_SNAPSHOT_ID`, `SparkReadConf.incrementalAppendScanBoundaries()`,
`SparkScanBuilder.buildIcebergIncrementalAppendScan()` — reachable today only
via the DataFrame reader. The missing piece is a SQL entry point.
`SparkSessionExtensions.injectTableFunction` (Spark 3.4+) is a public hook
that `IcebergSparkSessionExtensions` doesn't use yet (we currently inject a
parser, resolution rules, an optimizer rule, and a planner strategy). It lets
us add, with no Spark grammar change:
```sql
SELECT * FROM iceberg_incremental_changes('db.tbl', <startSnapshotId>,
<endSnapshotId>)
```
I traced the resolution path against Spark 4.2.0 to confirm it actually
reaches the incremental scan:
- `injectTableFunction` takes a `Seq[Expression] => LogicalPlan` builder.
- The builder returns `UnresolvedRelation(Seq("db", "tbl"), new
CaseInsensitiveStringMap({"start-snapshot-id": …, "end-snapshot-id": …}),
false)` — the public constructor carries the options map.
- `ResolveRelations` resolves it via `DataSourceV2Relation.create(table,
catalog, ident, options, timeTravel)` — the overload that carries options.
- `DataSourceV2Relation.options()` flows into
`SparkTable.newScanBuilder(CaseInsensitiveStringMap)`, and `SparkScanBuilder`
already branches on `Spark3Util.containsIncrementalOptions(options)` →
`incrementalAppendScanBoundaries()` → `buildIcebergIncrementalAppendScan()`.
So the function body is thin — it reuses the exact option plumbing the
DataFrame incremental read already uses. (I verified the API at every hop; I
haven't run it end-to-end — a small integration test closes the one remaining
link, that `ResolveRelations` forwards the relation's options into `create`.)
## Semantics
Match the established DataFrame incremental-append contract, as @ted-jenks
proposed: **start-exclusive, end-inclusive, appended rows only**; deletes /
overwrites / replaces are invisible. Changelog semantics (`_change_type`,
reporting deletes) stay out of scope — `create_changelog_view` / the `.changes`
metadata table already cover that. Choosing `IncrementalAppendScan` here is
what resolves @smaheshwar-pltr's ambiguity: one well-defined answer rather than
the append-vs-changelog fork.
On the id-vs-ref ambiguity: a TVF signature sidesteps the `BETWEEN` type
collision. I'd start with snapshot-id arguments and add a timestamp-range
overload as a follow-up, rather than probing both in one clause the way
`TimeTravel.AsOfVersion` does.
## Decision I'd like from a committer before writing code
1. Is a table-valued function the direction you want, or do you prefer
read-option / metadata-table-style access?
2. Function naming — `iceberg_incremental_changes`, or something aligned
with the existing `*_changes` / changelog vocabulary?
3. Do you want the headline `VERSION BETWEEN` SELECT syntax pursued upstream
in Spark in parallel, or dropped in favor of the TVF?
Happy to put up the TVF prototype as soon as there's a thumbs-up on
direction; I can also take the Spark-grammar angle to dev@ if that's preferred.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
