-
Notifications
You must be signed in to change notification settings - Fork 1k
Fix table definition for 'pgsql_replication_hostgroups' #5043
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
Conversation
JavierJF
commented
Jul 11, 2025
- Reduce 'check_type' range to uniquely supported check ('read_only').
- Added table upgrade procedure.
- Fixed missing space in table definition (display for '\G').
- Reduce 'check_type' range to uniquely supported check ('read_only'). - Added table upgrade procedure. - Fixed missing space in table definition (display for '\G').
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.
Summary of Changes
Hello @JavierJF, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request focuses on refining the schema and ensuring robust upgrades for the pgsql_replication_hostgroups
table. It narrows the allowed values for the check_type
column to only the currently supported option and introduces an automated, online upgrade mechanism to seamlessly migrate existing installations to the new schema definition.
Highlights
- Refined
check_type
Constraint: Thecheck_type
column in thepgsql_replication_hostgroups
table (both in configuration and runtime definitions) has been updated to strictly enforce'read_only'
as the only allowed value. This removes previously supported, but apparently unused or unsupported, options likeinnodb_read_only
or combined checks. - Automated Schema Upgrade Procedure: A new disk upgrade procedure has been implemented for the
pgsql_replication_hostgroups
table. This procedure detects if the table is using an older schema version (pre-3.0.2), renames the existing table, creates a new one with the updated definition, and then migrates the data, ensuring thecheck_type
column is correctly set to'read_only'
during the migration. - Integration into Bootstrap Process: The newly added schema upgrade logic for
pgsql_replication_hostgroups
is now integrated into theProxySQL_Admin
initialization process, ensuring that the table is automatically upgraded when the system starts up if an older version is detected.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
The code changes reduce the 'check_type' range for 'pgsql_replication_hostgroups' to uniquely supported check ('read_only'), add a table upgrade procedure, and fix a missing space in the table definition.
void ProxySQL_Admin::disk_upgrade_pgsql_replication_hostgroups() { | ||
configdb->execute("PRAGMA foreign_keys = OFF"); | ||
|
||
int rci = configdb->check_table_structure( | ||
const_cast<char*>("pgsql_replication_hostgroups"), | ||
const_cast<char*>(ADMIN_SQLITE_TABLE_PGSQL_REPLICATION_HOSTGROUPS_V3_0_1) | ||
); | ||
|
||
if (rci) { | ||
// upgrade is required | ||
proxy_warning("Detected version pre-3.0.2 of table 'pgsql_replication_hostgroups'\n"); | ||
proxy_warning("ONLINE UPGRADE of table 'pgsql_replication_hostgroups' in progress\n"); | ||
// drop any existing table with suffix _v301 | ||
configdb->execute("DROP TABLE IF EXISTS pgsql_replication_hostgroups_v301"); | ||
// rename current table to add suffix _v301 | ||
configdb->execute("ALTER TABLE pgsql_replication_hostgroups RENAME TO pgsql_replication_hostgroups_v301"); | ||
// create new table | ||
configdb->build_table( | ||
const_cast<char*>("pgsql_replication_hostgroups"), | ||
const_cast<char*>(ADMIN_SQLITE_TABLE_PGSQL_REPLICATION_HOSTGROUPS), | ||
false | ||
); | ||
// copy fields from old table | ||
configdb->execute( | ||
"INSERT INTO pgsql_replication_hostgroups(writer_hostgroup, reader_hostgroup, check_type, comment)" | ||
" SELECT writer_hostgroup, reader_hostgroup, 'read_only', comment FROM pgsql_replication_hostgroups_v301" | ||
); | ||
} | ||
|
||
configdb->execute("PRAGMA foreign_keys = ON"); | ||
} |
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.
The database migration logic should be wrapped in a transaction to ensure atomicity. If any of the configdb->execute()
or configdb->build_table()
calls fail, the database could be left in an inconsistent state. For example, if the build_table
call fails after the ALTER TABLE ... RENAME
has succeeded, the pgsql_replication_hostgroups
table will be missing, which could prevent ProxySQL from starting correctly. To ensure atomicity, the entire migration process within the if (rci)
block should be wrapped in a database transaction. This will ensure that all changes are committed together, or rolled back if an error occurs.
configdb->execute("PRAGMA foreign_keys = OFF");
int rci = configdb->check_table_structure(
const_cast<char*>("pgsql_replication_hostgroups"),
const_cast<char*>(ADMIN_SQLITE_TABLE_PGSQL_REPLICATION_HOSTGROUPS_V3_0_1)
);
if (rci) {
// upgrade is required
proxy_warning("Detected version pre-3.0.2 of table 'pgsql_replication_hostgroups'\n");
proxy_warning("ONLINE UPGRADE of table 'pgsql_replication_hostgroups' in progress\n");
configdb->execute("BEGIN TRANSACTION");
// drop any existing table with suffix _v301
configdb->execute("DROP TABLE IF EXISTS pgsql_replication_hostgroups_v301");
// rename current table to add suffix _v301
configdb->execute("ALTER TABLE pgsql_replication_hostgroups RENAME TO pgsql_replication_hostgroups_v301");
// create new table
configdb->build_table(
const_cast<char*>("pgsql_replication_hostgroups"),
const_cast<char*>(ADMIN_SQLITE_TABLE_PGSQL_REPLICATION_HOSTGROUPS),
false
);
// copy fields from old table
configdb->execute(
"INSERT INTO pgsql_replication_hostgroups(writer_hostgroup, reader_hostgroup, check_type, comment)"
" SELECT writer_hostgroup, reader_hostgroup, 'read_only', comment FROM pgsql_replication_hostgroups_v301"
);
configdb->execute("COMMIT");
}
configdb->execute("PRAGMA foreign_keys = ON");
Can one of the admins verify this patch? |
|