rdblue commented on issue #1485:
URL: https://github.com/apache/iceberg/issues/1485#issuecomment-699242399
I agree with most of what's been said about case 1. I would expect the
behavior to be a point in time, but that doesn't fit with what Spark actually
does for existing tables. And I think it is right to be concerned about a
behavior change.
But I don't think that case 1 is an area where Iceberg should make a choice.
Caching datasets and invalidating those datasets is implemented above the
source in Spark, so I think this is a concern that Spark should solve, not
Iceberg. If Spark chooses to invalidate cached data after a write, then that's
fine. Similarly, if a `REFRESH` also invalidates cached data, that high-level
choice is up to Spark (case 3). I don't think it would be a good idea for
Iceberg to call into Spark to refresh or invalidate, when Spark is calling to
Iceberg to perform an operation in the first place.
Case 2 is the one that I think we should worry about. A temporary view is
basically a named logical plan, like a `Dataset`, so I'll just refer to them
both as a "dataset".
The first question is where a dataset lies on the spectrum between a view
(always up to date) and an RDD (effectively immutable). I think that we
probably agree here that **a dataset should have view behavior**, because:
* A temporary view would have view behavior
* Spark currently has no guarantee that a dataset will give the same result
because the planner is used. Each addition to a Dataset results in a different
logical plan that gets resolved and optimized in isolation. For example, `df =
spark.table("t").filterExpr("date = '2020-09-25'")` could have refinements
`odds = df.filterExpr("category = 'odd')` and `evens = df.filterExpr("category
= 'even'")`. There is no guarantee that `odds.union(evens)` has any
relationship to `df` because the extra filters require building separate scans
at different times.
* Spark's v1 behavior for refresh is based on invalidating cached data about
a table and it can't cache what has not been loaded.
* Spark supports refreshing tables, which appears to also refresh existing
datasets.
If we agree on what the behavior should be, then I think that the current
caching catalog is correct. Because it reuses the same `Table` instance, all
datasets that have a reference from that catalog are in sync. Changes to that
table will cause it to be refreshed, and the `refresh` command from Spark also
works.
When not using the caching catalog, then the behavior would be that each
dataset is independent and fixed at load time. I don't think that's correct. So
we should implement a way for `SparkTable` to check whether it should refresh
the Iceberg table. Then we could have the same behavior regardless of caching.
And, we should also consider routing IcebergSource queries through a catalog so
they are covered by the same mechanism.
I think that agrees with all of @aokolnychyi's points, except for the last
one. I think it is important to have a consistent version for operations like
self-joins. I'd rather invest in a Spark catalog refresh system, although I
think the suggestion to refresh tables for each operation would work in the
short term.
Also, I didn't look much into the caching point. Implementing
`SparkTable.equals` sounds good to me.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
