You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/12/08 18:21:00 UTC

[GitHub] [spark] sunchao commented on a diff in pull request #38823: [SPARK-41290][SQL] Support GENERATED ALWAYS AS expressions for columns in create/replace table statements

sunchao commented on code in PR #38823:
URL: https://github.com/apache/spark/pull/38823#discussion_r1043667401


##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableProvider.java:
##########
@@ -93,4 +93,18 @@ default Transform[] inferPartitioning(CaseInsensitiveStringMap options) {
   default boolean supportsExternalMetadata() {
     return false;
   }
+
+  /**
+   * Returns true if the source supports defining generated columns upon table creation in SQL.
+   * When false: any create/replace table statements with a generated column defined in the table
+   * schema will throw an exception during analysis.
+   *

Review Comment:
   nit: add `<p>` between the paragraphs 



##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableProvider.java:
##########
@@ -93,4 +93,18 @@ default Transform[] inferPartitioning(CaseInsensitiveStringMap options) {
   default boolean supportsExternalMetadata() {
     return false;
   }
+
+  /**
+   * Returns true if the source supports defining generated columns upon table creation in SQL.
+   * When false: any create/replace table statements with a generated column defined in the table
+   * schema will throw an exception during analysis.
+   *
+   * A generated column is defined with syntax: {@code colName colType GENERATED ALWAYS AS (expr)}
+   * The generation expression is stored in the column metadata with key "generationExpression".
+   *
+   * Override this method to allow defining generated columns in create/replace table statements.
+   */
+  default boolean supportsGeneratedColumnsOnCreation() {

Review Comment:
   Yea I'm not sure if the `TableProvider` is a good place for this, since it's only used by data sources who don't have a real catalog. For instance, Iceberg doesn't use it in the general case I think, cc @aokolnychyi @RussellSpitzer 
   
   What about `TableCapability`? 



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org