cloud-fan commented on a change in pull request #25651: [SPARK-28948][SQL]
Support passing all Table metadata in TableProvider
URL: https://github.com/apache/spark/pull/25651#discussion_r325731051
##########
File path:
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableProvider.java
##########
@@ -36,26 +40,21 @@
public interface TableProvider {
/**
- * Return a {@link Table} instance to do read/write with user-specified
options.
+ * Return a {@link Table} instance to do read/write with the given table
metadata. The returned
+ * table must report the same schema and partitioning with the given table
metadata.
*
- * @param options the user-specified options that can identify a table, e.g.
file path, Kafka
- * topic name, etc. It's an immutable case-insensitive
string-to-string map.
- */
- Table getTable(CaseInsensitiveStringMap options);
-
- /**
- * Return a {@link Table} instance to do read/write with user-specified
schema and options.
- * <p>
- * By default this method throws {@link UnsupportedOperationException},
implementations should
- * override this method to handle user-specified schema.
- * </p>
- * @param options the user-specified options that can identify a table, e.g.
file path, Kafka
- * topic name, etc. It's an immutable case-insensitive
string-to-string map.
- * @param schema the user-specified schema.
- * @throws UnsupportedOperationException
+ * @param schema The schema of the table to load. If it's empty,
implementations should infer it.
+ * @param partitions The data partitioning of the table to load. If it's
empty, implementations
+ * should infer it.
+ * @param properties The properties of the table to load. It should be
sufficient to define and
+ * access a table. The properties map may be {@link
CaseInsensitiveStringMap}.
+ *
+ * @throws IllegalArgumentException if the implementation can't infer
schema/partitioning, or
+ * the given schema/partitioning doesn't
match the actual data
+ * schema/partitioning.
*/
- default Table getTable(CaseInsensitiveStringMap options, StructType schema) {
- throw new UnsupportedOperationException(
- this.getClass().getSimpleName() + " source does not support
user-specified schema");
- }
+ Table getTable(
+ Optional<StructType> schema,
+ Optional<Transform[]> partitions,
+ Map<String, String> properties);
Review comment:
I'd like to discuss how the API should look like. The current use cases
include
1. users only specify options, implementation needs to infer
schema/partitioning
2. users specify options and schema, implementation needs to infer
partitioning
3. users specify all the things.
Shall we create 3 methods or just create one single method like this?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
