smaheshwar-pltr commented on code in PR #3512:
URL: https://github.com/apache/iceberg-python/pull/3512#discussion_r3484932809
##########
pyiceberg/table/__init__.py:
##########
@@ -2263,6 +2341,180 @@ def count(self) -> int:
return res
+IAS = TypeVar("IAS", bound="IncrementalAppendScan", covariant=True)
+
+
+class IncrementalAppendScan(BaseScan):
+ """An incremental scan of a table's data that accumulates appended data
between two snapshots.
+
+ Args:
+ row_filter:
+ A string or BooleanExpression that describes the
+ desired rows
+ selected_fields:
+ A tuple of strings representing the column names
+ to return in the output dataframe.
+ case_sensitive:
+ If True column matching is case sensitive
+ options:
+ Additional Table properties as a dictionary of
+ string key value pairs to use for this scan.
+ limit:
+ An integer representing the number of rows to
+ return in the scan result. If None, fetches all
+ matching rows.
+ from_snapshot_id_exclusive:
+ Optional ID of the "from" snapshot, to start the incremental scan
from, exclusively. When the scan is
+ ultimately planned, this must not be None. The snapshot does not
need to be present in the table metadata
+ (it may have been expired), as long as it is the parent of some
ancestor of the "to" snapshot.
+ to_snapshot_id_inclusive:
+ Optional ID of the "to" snapshot, to end the incremental scan at,
inclusively.
+ Omitting it will default to the table's current snapshot.
+ """
+
+ from_snapshot_id_exclusive: int | None
+ to_snapshot_id_inclusive: int | None
+
+ def __init__(
+ self,
+ table_metadata: TableMetadata,
+ io: FileIO,
+ row_filter: str | BooleanExpression = ALWAYS_TRUE,
+ selected_fields: tuple[str, ...] = ("*",),
+ case_sensitive: bool = True,
+ from_snapshot_id_exclusive: int | None = None,
+ to_snapshot_id_inclusive: int | None = None,
+ options: Properties = EMPTY_DICT,
+ limit: int | None = None,
+ ):
+ super().__init__(
+ table_metadata=table_metadata,
+ io=io,
+ row_filter=row_filter,
+ selected_fields=selected_fields,
+ case_sensitive=case_sensitive,
+ options=options,
+ limit=limit,
+ )
+ self.from_snapshot_id_exclusive = from_snapshot_id_exclusive
Review Comment:
I've now moved to the following setup. Like the normal data scan,
`IncrementalAppendScan` is a plain value object: its constructor parameters are
exactly its public attributes — the invariant the inherited `BaseScan.update()`
relies on to return refined copies, so the builders below are thin `update()`
wrappers.
```py
class IncrementalAppendScan(BaseScan):
# range state (new) — a single `from` slot + an inclusive flag,
mirroring java/cpp:
from_snapshot_id: int | None
from_snapshot_inclusive: bool # False = exclusive (the default)
to_snapshot_id: int | None
branch: str | None
# + inherited BaseScan config: row_filter, selected_fields,
case_sensitive, options, limit
```
The factory's Spark-style `from_snapshot_id_exclusive` /
`to_snapshot_id_inclusive` kwargs map
onto those attributes (`from_snapshot_id` + `from_snapshot_inclusive`):
```py
# Factory (front door) — mirrors `table.scan(...)`; both range bounds
optional.
table.incremental_append_scan(
from_snapshot_id_exclusive: int | None = None, # start, exclusive;
default = oldest ancestor of `to`
to_snapshot_id_inclusive: int | None = None, # end, inclusive;
default = current snapshot
row_filter: str | BooleanExpression = ALWAYS_TRUE,
selected_fields: tuple[str, ...] = ("*",),
case_sensitive: bool = True,
options: Properties = EMPTY_DICT,
limit: int | None = None,
) -> IncrementalAppendScan
# Builders (power surface) — each returns a refined copy, like
select()/filter().
IncrementalAppendScan.from_snapshot_id_exclusive(id) /
.from_snapshot_id_inclusive(id)
.to_snapshot_id_inclusive(id)
.use_branch(name)
# .from_ref_* / to_ref_* — start/end by ref, follow-up
# + inherited select() / filter() /
with_case_sensitive() / update()
```
Choosing `from_snapshot_id_exclusive` / `to_snapshot_id_inclusive` instead
of `set/update_snapshot_id_exclusive` matches the existing flow of `filter()` /
`select()` naming for normal data scans and makes the chaining above read more
smoothly, IMO.
(Note: I've left out `branch` / `use_branch` to be added as a follow-up
together with appropriate tests, because that makes sense to me as a splitting
point here)
--
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]
