Re: [I] Support version/timestamp range queries in Spark SQL syntax [iceberg]

["hreddygayam (via GitHub)" <gi...@apache.org>]

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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org