-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Spark 3.4: IcebergSource extends SessionConfigSupport #7732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
b21335a
2dddaa7
9558bad
f9d8ca4
d3b9f9d
ea7a515
0b5ded7
82bc24b
5ee5cbf
ff515b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -154,6 +154,10 @@ spark.read | |||
| .table("catalog.db.table") | ||||
| ``` | ||||
|
|
||||
| Iceberg 1.8.0 and later support setting read options by Spark session configuration `spark.datasource.iceberg.<key>=<value>` | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still think we need new section like 'Configuration Priority' where we can explain the order of precedence:
DataFrame Reads:
(please double check)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hesitate to write such a section because the situation looks more complex, some configurations are allowed to be set by dedicated session configuration, for example |
||||
| when using DataFrame to read Iceberg tables, for example: `spark.datasource.iceberg.split-size=512m`, it has lower priority | ||||
| than options explicitly passed to DataFrameReader. | ||||
|
|
||||
| | Spark option | Default | Description | | ||||
| | --------------- | --------------------- | ----------------------------------------------------------------------------------------- | | ||||
| | snapshot-id | (latest) | Snapshot ID of the table snapshot to read | | ||||
|
|
@@ -167,16 +171,20 @@ spark.read | |||
|
|
||||
| ### Write options | ||||
|
|
||||
| Spark write options are passed when configuring the DataFrameWriter, like this: | ||||
| Spark write options are passed when configuring the DataFrameWriterV2, like this: | ||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I replaced the example with iceberg/docs/docs/spark-writes.md Line 264 in 319f29e
|
||||
|
|
||||
| ```scala | ||||
| // write with Avro instead of Parquet | ||||
| df.write | ||||
| df.writeTo("catalog.db.table") | ||||
| .option("write-format", "avro") | ||||
| .option("snapshot-property.key", "value") | ||||
| .insertInto("catalog.db.table") | ||||
| .append() | ||||
| ``` | ||||
|
|
||||
| Iceberg 1.8.0 and later support setting write options by Spark session configuration `spark.datasource.iceberg.<key>=<value>` | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we extract to its own section, no need to repeat it?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I write it here because it's "Write options", actually, Spark has different concepts to allow the format/extensions to control the behavior, i.e. table properties, session configurations, options. |
||||
| when using DataFrame to write Iceberg tables, for example: `spark.datasource.iceberg.write-format=orc`, it has lower priority | ||||
| than options explicitly passed to DataFrameWriterV2. | ||||
|
|
||||
| | Spark option | Default | Description | | ||||
| | ---------------------- | -------------------------- | ------------------------------------------------------------ | | ||||
| | write-format | Table write.format.default | File format to use for this write operation; parquet, avro, or orc | | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ | |
| import org.apache.spark.sql.connector.catalog.CatalogManager; | ||
| import org.apache.spark.sql.connector.catalog.CatalogPlugin; | ||
| import org.apache.spark.sql.connector.catalog.Identifier; | ||
| import org.apache.spark.sql.connector.catalog.SessionConfigSupport; | ||
| import org.apache.spark.sql.connector.catalog.SupportsCatalogOptions; | ||
| import org.apache.spark.sql.connector.catalog.Table; | ||
| import org.apache.spark.sql.connector.catalog.TableCatalog; | ||
|
|
@@ -61,7 +62,8 @@ | |
| * <p>The above list is in order of priority. For example: a matching catalog will take priority | ||
| * over any namespace resolution. | ||
| */ | ||
| public class IcebergSource implements DataSourceRegister, SupportsCatalogOptions { | ||
| public class IcebergSource | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another comment, is it now multiple ways to configure properties (including #4011), it may be confusing to user. Worth to add a documentation about it, listing the precedence, ie: I guess using dataframe API (to be double-checked)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I updated the docs and hope it's clear now. |
||
| implements DataSourceRegister, SupportsCatalogOptions, SessionConfigSupport { | ||
| private static final String DEFAULT_CATALOG_NAME = "default_iceberg"; | ||
| private static final String DEFAULT_CACHE_CATALOG_NAME = "default_cache_iceberg"; | ||
| private static final String DEFAULT_CATALOG = "spark.sql.catalog." + DEFAULT_CATALOG_NAME; | ||
|
|
@@ -80,6 +82,11 @@ public String shortName() { | |
| return "iceberg"; | ||
| } | ||
|
|
||
| @Override | ||
| public String keyPrefix() { | ||
| return shortName(); | ||
| } | ||
|
|
||
| @Override | ||
| public StructType inferSchema(CaseInsensitiveStringMap options) { | ||
| return null; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good, but was also thinking of adding a section for priority as well as mentioned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be in its own section, like "session level configuration"?