rdblue commented on pull request #31475:
URL: https://github.com/apache/spark/pull/31475#issuecomment-775490751
@MaxGekk, can you share the use case that you have for this? You mentioned
truncation-specific optimizations. I think working with concrete use cases is
usually a good idea. If these are theoretical only -- like a user that can drop
all data but not a subset -- then we should put this off. If there's a specific
case, then let's discuss it.
I agree that there _may_ be good reason to pass that the engine's intent was
to truncate. That's why we have `SupportsTruncate` for the write builder. And I
agree with you that we don't necessarily need to use an atomic operation that
could truncate and add data at the same time. Your point about not having
insert permissions is a good one to justify not using `SupportsTruncate`,
although the case of a user that can drop all data but not subsets doesn't
sound real. The point about truncation possibly being a metadata operation is
why we added `SupportsDelete` at the table level.
Those points may indicate that an interface to truncate a table as a
stand-alone operation is valid, although I still think that it is a bad idea to
add more interfaces to v2 without a reasonable expectation that they will
actually be used.
Another problem here is that this is operation is proposed at the catalog
level, which does not fit with how v2 works. I think that the reason for this
is emulating what the Hive does, but that's not usually a good choice.
In v2, catalogs load tables and tables are modified. That's why
`SupportsDelete` extends `Table` and not `TableCatalog`. This keeps concerns
separate, so we have a way to handle tables that don't exist and a separate way
to handle tables that don't support a certain operation. Mixing those two
together at the catalog level over-complicates the API, requiring a source to
throw one exception if the table doesn't exist and another if it doesn't
support truncation. (We also went through this discussion with the recently
added interfaces to add/drop partitions.)
Assuming that it is worth adding this interface, I would expect it to be a
mix-in for `Table`. And like `SupportsOverwrite` that implements
`SupportsTruncate`, I think this should also update `SupportsDelete` so that
tables don't need to implement both interfaces.
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
