Wenchen, sorry for the delay, I will post an update shortly. On Thu, Sep 3, 2020 at 2:00 AM Wenchen Fan <cloud0...@gmail.com> wrote:
> Any updates here? I agree that a new View API is better, but we need a > solution to avoid performance regression. We need to elaborate on the cache > idea. > > On Thu, Aug 20, 2020 at 7:43 AM Ryan Blue <rb...@netflix.com> wrote: > >> I think it is a good idea to keep tables and views separate. >> >> The main two arguments I’ve heard for combining lookup into a single >> function are the ones brought up in this thread. First, an identifier in a >> catalog must be either a view or a table and should not collide. Second, a >> single lookup is more likely to require a single RPC. I think the RPC >> concern is well addressed by caching, which we already do in the Spark >> catalog, so I’ll primarily focus on the first. >> >> Table/view name collision is unlikely to be a problem. Metastores that >> support both today store them in a single namespace, so this is not a >> concern for even a naive implementation that talks to the Hive MetaStore. I >> know that a new metastore catalog could choose to implement both >> ViewCatalog and TableCatalog and store the two sets separately, but that >> would be a very strange choice: if the metastore itself has different >> namespaces for tables and views, then it makes much more sense to expose >> them through separate catalogs because Spark will always prefer one over >> the other. >> >> In a similar line of reasoning, catalogs that expose both views and >> tables are much more rare than catalogs that only expose one. For example, >> v2 catalogs for JDBC and Cassandra expose data through the Table interface >> and implementing ViewCatalog would make little sense. Exposing new data >> sources to Spark requires TableCatalog, not ViewCatalog. View catalogs are >> likely to be the same. Say I have a way to convert Pig statements or some >> other representation into a SQL view. It would make little sense to combine >> that with some other TableCatalog. >> >> I also don’t think there is benefit from an API perspective to justify >> combining the Table and View interfaces. The two share only schema and >> properties, and are handled very differently internally — a View’s SQL >> query is parsed and substituted into the plan, while a Table is wrapped in >> a relation that eventually becomes a Scan node using SupportsRead. A view’s >> SQL also needs additional context to be resolved correctly: the current >> catalog and namespace from the time the view was created. >> >> Query planning is distinct between tables and views, so Spark doesn’t >> benefit from combining them. I think it has actually caused problems that >> both were resolved by the same method in v1: the resolution rule grew >> extremely complicated trying to look up a reference just once because it >> had to parse a view plan and resolve relations within it using the view’s >> context (current database). In contrast, John’s new view substitution rules >> are cleaner and can stay within the substitution batch. >> >> People implementing views would also not benefit from combining the two >> interfaces: >> >> - There is little overlap between View and Table, only schema and >> properties >> - Most catalogs won’t implement both interfaces, so returning a >> ViewOrTable is more difficult for implementations >> - TableCatalog assumes that ViewCatalog will be added separately like >> John proposes, so we would have to break or replace that API >> >> I understand the initial appeal of combining TableCatalog and ViewCatalog >> since it is done that way in the existing interfaces. But I think that Hive >> chose to do that mostly on the fact that the two were already stored >> together, and not because it made sense for users of the API, or any other >> implementer of the API. >> >> rb >> >> On Tue, Aug 18, 2020 at 9:46 AM John Zhuge <jzh...@apache.org> wrote: >> >>> >>> >>> >>>> > AFAIK view schema is only used by DESCRIBE. >>>> >>>> Correction: Spark adds a new Project at the top of the parsed plan from >>>> view, based on the stored schema, to make sure the view schema doesn't >>>> change. >>>> >>> >>> Thanks Wenchen! I thought I forgot something :) Yes it is the validation >>> done in *checkAnalysis*: >>> >>> // If the view output doesn't have the same number of columns >>> neither with the child >>> // output, nor with the query column names, throw an >>> AnalysisException. >>> // If the view's child output can't up cast to the view output, >>> // throw an AnalysisException, too. >>> >>> The view output comes from the schema: >>> >>> val child = View( >>> desc = metadata, >>> output = metadata.schema.toAttributes, >>> child = parser.parsePlan(viewText)) >>> >>> So it is a validation (here) or cache (in DESCRIBE) nice to have but not >>> "required" or "should be frozen". Thanks Ryan and Burak for pointing that >>> out in SPIP. I will add a new paragraph accordingly. >>> >> >> >> -- >> Ryan Blue >> Software Engineer >> Netflix >> > -- John Zhuge
