-
Notifications
You must be signed in to change notification settings - Fork 25
Introduce the 'partman_5_compatibility_mode' config option #138
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: partman-migration
Are you sure you want to change the base?
Conversation
Co-authored-by: George Radecke <[email protected]>
- `prefer_single_step_column_addition_with_default`: If true, raise an error when adding a column and separately setting a constant default value for that column in the same migration. Default: `true` | ||
- `allow_force_create_table`: If false, the `force: true` option to ActiveRecord's `create_table` method is disallowed. Default: `false` | ||
- `infer_primary_key_on_partitioned_tables`: If true, the primary key for partitioned tables will be inferred on PostgreSQL 11+ databases (identifier column + partition key columns). Default: `true` | ||
- `partman_5_compatibility_mode`: If true, `safe_partman_create_parent` will raise an error if the user provides an interval that is [not supported by Partman 5](https://github.com/pgpartman/pg_partman/blob/v5.2.4/sql/functions/create_parent.sql#L86-L96). If the interval is supported, the method will ensure table name suffixes match the Partman 5 format (`YYYYMMDD`, `YYYYMMDD_HTH24MISS`). Default: `false` |
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.
When will we plan to flip this to true
? I'm not sure we'd want to wait until a major version of this gem, but...
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.
In reality, I wouldn't expect there to be a ton of users in the community utilizing the partman functionality in this gem. On the BT side of things, I'm imagining we would flip this to true
in our initializers very soon. So I think we should just follow semantic versioning and wait until 3.0 to flip the default to true
. Honestly, in 3.0 we could probably just remove this config entirely and drop support for partman 4
end | ||
|
||
describe "config" do | ||
after(:each) do |
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.
This is unrelated cleanup, right?
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.
Sort of. We moved this hook into the spec helper so the config universally gets reset after each test. We did this so we could easily set PgHaMigrations.config.partman_5_compatibility_mode = true
in our new tests. However... looking more closely, it appears that other tests simply stub out values for config options. Should we follow that pattern and revert this change?
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.
Should we follow that pattern and revert this change?
We decided that, yes, we should. Pushed up a commit to address
Keyword intervals used in Partman 4 will not be available in Partman 5.
See: #137
To assist teams with the transition to Partman 5, we have added a configuration option to enable compatibility mode. When this is enabled, a migration that calls
safe_partman_create_parent
with an interval not supported by Partman 5 will raise an error. If a supported interval is provided, but the underlying extension is Partman 4, the table suffixes anddatetime_string
will be adjusted to align with Partman 5. When this option is disabled (the default), existing behavior of this method is preserved for backwards compatibility.