Antalya-26.3: Resolve currentDatabase() in Hybrid segment metadata#1995
Antalya-26.3: Resolve currentDatabase() in Hybrid segment metadata#1995mkmkme wants to merge 4 commits into
Conversation
A Hybrid table created in a non-default database with `currentDatabase()` in an additional segment persisted the unresolved `currentDatabase()` token in its metadata. On restart the startup ATTACH replays the definition with no session database, so `currentDatabase()` resolved to `default` and the segment pointed at a missing `default` table, failing to attach with `UNKNOWN_TABLE`. `registerStorageHybrid` resolved `currentDatabase()` only on the clone used for schema validation and in-memory segment execution, never on the original `engine_args[i]` that gets serialized to metadata. Resolve it on the original argument too, so the persisted value is a concrete database name. This mirrors the base segment, which is already resolved in place by `TableFunctionRemote::parseArguments`, and the identifier branch, which writes the qualified name back into `engine_args[i]`. Add an integration test that creates a Hybrid table in a non-default database using `currentDatabase()`, restarts the server, and verifies the table still attaches and is queryable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ba1a9d383
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The reference file referred to currentDatabase(), replace it with plain 'default' instead
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1037c7d712
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Normalize arguments (evaluate `currentDatabase()`, expand named collections, etc.). | ||
| // TableFunctionFactory::get mutates the AST in-place inside TableFunctionRemote::parseArguments. | ||
| ASTPtr normalized_table_function_ast = table_function_ast->clone(); | ||
| replaceCurrentDatabaseFunction(engine_args[i], local_context); |
There was a problem hiding this comment.
This new metadata rewrite only catches AST functions named exactly currentDatabase, but the same function is exposed through accepted aliases such as DATABASE, SCHEMA, and current_database. For an additional segment like remote('localhost:9000', DATABASE(), 'local_cold'), TableFunctionRemote::parseArguments still normalizes the clone used in memory, while the original engine_args[i] keeps the unresolved alias in the stored definition, so reattach/startup from a different current database can still point the segment at the wrong database and raise the same UNKNOWN_TABLE exception this patch is fixing. Please normalize the actual remote/cluster database argument with the existing database-name constant evaluator instead of relying on this exact-name tree walk.
Useful? React with 👍 / 👎.
2 participants
A Hybrid table created in a non-default database with
currentDatabase()in an additional segment persisted the unresolvedcurrentDatabase()token in its metadata. On restart the startup ATTACH replays the definition with no session database, socurrentDatabase()resolved todefaultand the segment pointed at a missingdefaulttable, failing to attach withUNKNOWN_TABLE.registerStorageHybridresolvedcurrentDatabase()only on the clone used for schema validation and in-memory segment execution, never on the originalengine_args[i]that gets serialized to metadata. Resolve it on the original argument too, so the persisted value is a concrete database name. This mirrors the base segment, which is already resolved in place byTableFunctionRemote::parseArguments, and the identifier branch, which writes the qualified name back intoengine_args[i].Add an integration test that creates a Hybrid table in a non-default database using
currentDatabase(), restarts the server, and verifies the table still attaches and is queryable.Fixes #1993
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fixes an issue where
currentDatabase()used as a hybrid table argument would cause anUNKNOWN_TABLEerror on restartDocumentation entry for user-facing changes
...
CI/CD Options
Exclude tests:
Regression jobs to run: