Antalya-26.3: Resolve currentDatabase() in Hybrid segment metadata - #1995
Open
mkmkme wants to merge 7 commits into
Open
Antalya-26.3: Resolve currentDatabase() in Hybrid segment metadata#1995mkmkme wants to merge 7 commits into
mkmkme wants to merge 7 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>
Collaborator
Author
|
@codex review |
filimonov
reviewed
Jul 1, 2026
![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
Collaborator
Author
|
@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".
… metadata `replaceCurrentDatabaseFunction` compared the AST function name literally, so it missed the case-insensitive aliases of `currentDatabase` — `DATABASE`, `SCHEMA` and `current_database`. Engine arguments never go through `FunctionNameNormalizer` (it skips the engine clause of `ASTStorage` on purpose), so the alias survives in the AST and reaches the metadata unresolved. The in-memory clone still worked, because `TableFunctionRemote::parseArguments` evaluates the database argument through the function factory, which knows the aliases. That made the gap invisible until a restart, where the alias resolved to `default` and the segment failed to attach with `UNKNOWN_TABLE` — the exact bug this branch fixes for the plain `currentDatabase()` spelling. Resolve the name through `getFunctionCanonicalNameIfAny` and cover the alias spelling in `04302_hybrid_current_database_reattach`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The tag reason was "requires remote() table function", but the fast test runs a
normal local server, so `remote('localhost:9000', ...)` works there. Ten of the
twelve stateless tests that use `remote('localhost:9000', ...)` carry no tag,
including `03643_hybrid`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
30 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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: