morningman opened a new pull request, #69086:
URL: https://github.com/apache/doris/pull/69086
### What problem does this PR solve?
Issue Number: None
Related PR: #66428
Problem Summary:
**In short.** This ports #66428 from branch-4.1 (it is also in branch-4.2):
`ALTER TABLE ... SET ("key" = "value", ...)` sets the native properties of an
Iceberg or Paimon table. On master the statement fails analysis for any key
other than `auto_analyze_policy`. This PR adds a connector SPI method for it,
implements it for Iceberg and Paimon, and commits all the keys of one statement
together or none of them.
**Background.** On master every external catalog is plugin driven. An `ALTER
TABLE` on an external table reaches `PluginDrivenExternalCatalog`, which
resolves the connector's table handle and calls the matching
`ConnectorMetadata` method (column evolution, branches and tags, partition
evolution). The only `SET (...)` an external table accepted was
`auto_analyze_policy`, which Doris stores itself. In branch-4.x, #66428 added
`updateTableProperties` to the Iceberg and Paimon metadata ops in fe-core.
master has no such ops in fe-core, and the SPI had no method for table
properties.
**The problem, and what it cost.**
| Statement on an Iceberg or Paimon table | master | with this PR |
|---|---|---|
| `SET ('write.target-file-size-bytes' = '134217728')` | `Unknown table
property: [write.target-file-size-bytes]` | set on the table |
| `SET ('snapshot.num-retained.min' = '3', 'snapshot.num-retained.max' =
'6')` | `Can only set one table property(without dynamic partition && binlog)
at a time` | both set, as one Paimon schema change |
| `SET ('auto_analyze_policy' = 'disable')` | stored by Doris | unchanged |
| `SET ('auto_analyze_policy' = 'enable', 'write.target-file-size-bytes' =
'536870912')` | `Can only set one table property ...` | `auto_analyze_policy
cannot be set with external table properties` |
Properties such as snapshot retention, target file size or manifest merging
could only be changed from another engine, such as Spark or Flink.
**How this PR fixes it.**
- SPI: `ConnectorTableDdlOps#setTableProperties(session, handle,
properties)`. The default throws `SET TABLE PROPERTIES not supported`. A
connector that implements it must commit every entry of the map or none of
them. The recorded method surface is updated.
- fe-core, as in branch-4.1: `ModifyTablePropertiesOp` accepts any number of
keys for a table outside the internal catalog and rejects `auto_analyze_policy`
mixed with other keys. `Alter` keeps `auto_analyze_policy` on the Doris path
and sends the other keys to `CatalogIf#updateTableProperties`.
`PluginDrivenExternalCatalog` calls the connector, and writes the refresh edit
log and refreshes the cached table only after the connector returns.
- Iceberg: one `UpdateProperties` commit inside the catalog's auth context.
Iceberg validates the whole map before it commits, so a map with a
format-version downgrade does not commit its valid keys either.
- Paimon: one `Catalog.alterTable` with a `SchemaChange.setOption` per key.
Paimon validates the whole list before it commits, and one statement adds one
schema version.
- HMS: as it does for the other ALTER operations, the HMS gateway forwards
the call to its Iceberg sibling for an Iceberg table, so the statement behaves
the same through an HMS catalog. A Hive table still reports it as unsupported.
Branch-4.x does not support it through an HMS catalog.
**Results.**
- Unit tests:
- `ModifyTablePropertiesOpTest`, from branch-4.1: validation for external
and internal tables, and of `auto_analyze_policy`.
- `PluginDrivenExternalCatalogDdlRoutingTest`: the whole map reaches the
connector in one call, and the refresh edit log and cache refresh follow. A
connector failure is reported, and neither of them happens.
- `CatalogBackedIcebergCatalogOpsDdlTest`, against an in-memory Iceberg
catalog: two keys add one metadata version, a later SET keeps the keys it does
not name, and a map with a downgrade commits nothing.
- `PaimonConnectorMetadataTablePropertiesTest`, against a local filesystem
catalog: two options add one schema version and keep the options they do not
name, and a list with an invalid field option commits nothing.
- `IcebergConnectorMetadataDdlTest`,
`HiveConnectorMetadataSiblingDelegationTest` and `ConnectorMetadataSurfaceTest`
cover the auth wrapping, the HMS forwarding and the SPI surface.
- Each of these changes turned the matching tests red: one alter per
Paimon option, one commit per Iceberg key, and refreshing before the connector
call. The full suites of fe-connector-spi, -hive, -iceberg and -paimon pass.
- Regression, on a local cluster with the Iceberg docker environment (REST
catalog, MinIO, Spark):
- `test_iceberg_alter_properties` reads the properties back through Spark
after each statement.
- `test_paimon_alter_properties` checks `$options` and `$schemas`. Its
`.out` was generated there: the schema id goes 0, 1, 2 for the two SETs and
stays at 2 after the `auto_analyze_policy` statement and after the rejected one.
- Both suites come from branch-4.1. The Paimon one records its results in
a `.out` instead of asserting them.
Original author: @suxiaogang223
### Release note
Support `ALTER TABLE ... SET (...)` to change the native properties of
Iceberg and Paimon tables. Several properties can be set in one statement, and
they are committed together.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- `test_iceberg_alter_properties` and `test_paimon_alter_properties`
(external_table_p0).
- [x] Unit Test
- `ModifyTablePropertiesOpTest`,
`PluginDrivenExternalCatalogDdlRoutingTest`,
`CatalogBackedIcebergCatalogOpsDdlTest`, `IcebergConnectorMetadataDdlTest`,
`PaimonConnectorMetadataTablePropertiesTest`,
`HiveConnectorMetadataSiblingDelegationTest`, `ConnectorMetadataSurfaceTest`.
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
- `ALTER TABLE ... SET (...)` on an Iceberg or Paimon table sets the
table's native properties instead of failing analysis.
- Does this need documentation?
- [ ] No.
- [x] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
- The Iceberg and Paimon catalog pages do not describe `ALTER TABLE
... SET (...)` yet.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
