-
Notifications
You must be signed in to change notification settings - Fork 843
feat: database level default connection configuration #18886
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
base: main
Are you sure you want to change the base?
feat: database level default connection configuration #18886
Conversation
e649fb2 to
f778e9b
Compare
fe1a632 to
1d719d7
Compare
|
@codex review |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
drmingdrmer
left a comment
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.
@drmingdrmer reviewed 7 of 27 files at r1, all commit messages.
Reviewable status: 7 of 27 files reviewed, 1 unresolved discussion (waiting on @dantengsky and @SkyFan2002)
src/meta/api/src/database_api.rs line 545 at r1 (raw file):
.await? .map(|v| v.seq()) .unwrap_or_default();
re-fetching it may not get the correct version of the existing record. transistion contains the existing value. Use the following instead:
transition.prev.map(|seq_v| seq_v.seq)
see:
databend/src/meta/types/src/change.rs
Line 32 in 96108c4
| pub struct Change<T, ID = u64> |
Code quote:
if !transition.is_changed() {
let curr_seq = self
.get_pb(&db_key)
.await?
.map(|v| v.seq())
.unwrap_or_default();
drmingdrmer
left a comment
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.
@drmingdrmer reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: 7 of 27 files reviewed, 1 unresolved discussion (waiting on @dantengsky and @SkyFan2002)
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
Two New Database-Level Options
DEFAULT_STORAGE_CONNECTION,DEFAULT_STORAGE_PATHThese options define the default storage connection and path for the database. Any table created in the database without explicit storage settings will inherit these database-level defaults.
You can set the options when creating the database, or adjust them later with
ALTER DATABASE.Example
Notes:
When creating a database you cannot supply only one of
DEFAULT_STORAGE_CONNECTIONorDEFAULT_STORAGE_PATH; they must appear together or both be omitted.When altering an existing database you may change one option at a time as long as the other option already exists; otherwise the command fails.
During
CREATE DATABASE/ALTER DATABASE, connection reachability and storage path permissions are validated.Updating these database-level options does not change storage parameters of existing tables; only tables created afterwards use the new defaults.
fixes: Feature Request: Database level default connection configuration #18825
Tests
Type of change
This change is