morningman opened a new pull request, #69228:
URL: https://github.com/apache/doris/pull/69228
### What problem does this PR solve?
Issue Number: None
Related PR: #67163
Problem Summary:
**In short.** Ports the rest of branch-4.1 #67163 to master. Iceberg JDBC
and Glue catalogs accept `CREATE DATABASE ... PROPERTIES (...)`, and Glue also
accepts `location`. A Paimon `ALTER TABLE ... SET` that names `path` is
rejected before anything is committed.
**Background.** #67163 aligned database properties with what each catalog
type can store and use, and rejected the Paimon `path` option in `ALTER TABLE
... SET`. Master has the Paimon database part since #67395
(`PaimonConnectorMetadata.createDatabase`: HMS, JDBC, REST and DLF store
properties, only HMS and DLF use `location`). The Iceberg part was not ported,
and Paimon `ALTER TABLE ... SET` came to master later, with #69086, without the
`path` check.
**The problem, and what it cost.**
- Iceberg accepts database properties only for HMS catalogs. On a JDBC or
Glue catalog, which store namespace properties, `CREATE DATABASE db PROPERTIES
('owner' = 'doris')` fails with `Not supported: create database with properties
for iceberg catalog type: jdbc`.
- `ALTER TABLE paimon_t SET ('path' = '...')` commits a new schema version
that stores the new path. Paimon leaves that check to the engine
(`SchemaManager.checkAlterTablePath`, which nothing in paimon-core calls). The
table does not move: its files stay where they are, and the catalog keeps
opening it there, so the stored path is wrong metadata, shown by `$options`.
**How this PR fixes it.**
- `IcebergConnectorMetadata.createDatabase` checks the catalog type the way
the Paimon connector does:
| Iceberg catalog type | Database properties | `location` |
|---|---|---|
| HMS, Glue | Accepted, forwarded unchanged | Accepted |
| JDBC | Accepted, forwarded unchanged | Rejected: the catalog would store
it but not place new tables under it |
| Hadoop, REST, DLF, S3 Tables | Rejected, as before (a REST server may
ignore namespace properties) | Rejected |
- `PaimonConnectorMetadata.setTableProperties` runs every key through
`SchemaManager.checkAlterTablePath` before the remote alter. A `path` entry, in
any case, fails with `Change path is not supported yet.`, and the other entries
of the statement are not committed either.
**Results.** On a local cluster with the Iceberg docker environment, and
PostgreSQL 14 and MySQL 5.7 for the Iceberg JDBC catalog:
| Statement | Before | After |
|---|---|---|
| Iceberg JDBC (PostgreSQL): `CREATE DATABASE db PROPERTIES ('owner' =
'doris', 'team' = 'lakehouse')` | `Not supported: create database with
properties for iceberg catalog type: jdbc` | Created;
`iceberg_namespace_properties` has `owner` and `team` |
| Iceberg JDBC: `CREATE DATABASE db PROPERTIES ('location' = 's3://...')` |
Same error | `Not supported: database property 'location' for iceberg catalog
type: jdbc because it does not determine the default table location` |
| Paimon: `ALTER TABLE t SET ('PATH' = 's3://warehouse/relocated_table')` |
New schema version whose options store the new path; data stays in place |
`Change path is not supported yet.`; schema version and `$options` unchanged |
The Paimon "before" row is what paimon-core does with that `SetOption` on a
local `FileSystemCatalog`: schema 1 stores `path = <new location>`, while the
files and the location the catalog opens stay where they were.
### Release note
Iceberg JDBC and Glue catalogs accept CREATE DATABASE with properties, and
Glue also accepts the location property. ALTER TABLE ... SET on a Paimon table
rejects the path option.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- [x] Unit Test
- [ ] 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? -->
- `IcebergConnectorMetadataDdlTest`: the catalog-type matrix for
properties and `location`. `PaimonConnectorMetadataTablePropertiesTest`: `path`
is rejected before the remote alter, and on a local catalog the stored schema
keeps its version and gets no `path`. With the previous code, three of the four
new Iceberg cases and both Paimon cases fail; with the check moved after the
alter, both Paimon cases fail.
- Regression: `test_iceberg_jdbc_catalog` (PostgreSQL and MySQL
backends) and `test_paimon_alter_properties` pass on the local cluster; the
`.out` of the latter was generated by the script.
- Behavior changed:
- [ ] No.
- [x] Yes. Iceberg JDBC and Glue catalogs accept database properties
(Glue also `location`); Paimon rejects `ALTER TABLE ... SET ('path' = ...)`.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### 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]
