feat: Derive pg_partman partition width from ttl unit and partition-divisor setting - #2236
feat: Derive pg_partman partition width from ttl unit and partition-divisor setting#2236WellMafra wants to merge 4 commits into
Conversation
… in ttl hint Signed-off-by: Wellington Mafra <wellingtonisidiomafra@gmail.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2236 +/- ##
============================================
+ Coverage 16.91% 17.93% +1.01%
- Complexity 1040 1105 +65
============================================
Files 613 613
Lines 17738 17802 +64
Branches 2155 2161 +6
============================================
+ Hits 3001 3193 +192
+ Misses 14425 14291 -134
- Partials 312 318 +6 ☔ View full report in Codecov by Harness. |
Signed-off-by: Wellington Mafra <wellingtonisidiomafra@gmail.com>
Signed-off-by: Wellington Mafra <wellingtonisidiomafra@gmail.com>
ferenc-csaky
left a comment
There was a problem hiding this comment.
I don't think this solution is viable as is. The ttl hint is orthogonal to the external system, and now we are introducing a PG-specific parameter to it, which violates the contract of this hint, i.e. it works regardless what external data system connection that table has. Cause TTL is also meaningful for Kafka for example.
It makes sense to control this on the table level, but i may add a new pg_partman hint for this maybe?
Or another idea to extend the engine hint to engine-specific configs, which is a more general construct, but would solve this problem too.
cc @mbroecheler
cc6e80c to
3a36c73
Compare
|
@WellMafra I agree with @ferenc-csaky reasoning. We don't want this engine specific logic in the generic TTL hint. Moreover, I feel like it is too much to ask our users to fine-tune this variable. Instead, we should have better default handling that works for 99% of cases. Here is what I propose: We have a postgres specific This allows the user to control the number of partitions (via config option) and the floor (via the unit they use). The default value for That means, a |
Reworked per review feedback from @ferenc-csaky and @mbroecheler: instead of adding a PG-specific second argument to the engine-agnostic
ttlhint, the pg_partman partition width is now derived automatically from the TTL duration and a postgres-specificpartition-divisorengine setting.How the partition width is picked
For range-partitioned tables with
ttl(X unit)and the postgres settingpartition-divisor(P, default 100):The divisor caps the partition count for long TTLs, while the unit the user declares the TTL with sets the floor:
ttl(14 days)produces 1-day partitions,ttl(48 hours)produces hourly partitions. Retention is still always the TTL duration.Key Changes
TtlHintstays a single-argument hint and now captures the declared unit. Units are validated to be between minute and week (min,hour,day,week+ short/plural forms); seconds and below or months and above raiseStatementParserExceptionwith the source location. Thecache(...)hint is unchanged.partition-divisor(default100), read inPostgresJdbcEngineand validated as a positive integer.AbstractJdbcStatementFactorygains an overridablederivePartitionInterval(partitionType, ttl, ttlUnit)hook (null by default);PostgresStatementFactoryimplements the formula above for RANGE tables.CreateTableJdbcStatement.partitionIntervalnow holds the computed width (still omitted from JSON when null);PgPartmanExtensionuses it directly and the old TTL-bucketingderiveIntervalfallback is removed.sqrl-language.mdand thepartition-divisorsetting + partitioning behavior inconfiguration-engine/postgres.md.TtlHintTestrewritten for unit parsing/validation, newPostgresStatementFactoryTestcovering the formula (floor dominance, divisor scaling, menu snapping, smallest-entry fallback, config parsing), updatedPgPartmanExtensionTest, andpartitionTest.sqrlextended with attl(48 hours)table (snapshot regenerated).Behavior change
Derived partition widths change for existing TTLs: e.g.
ttl(30 days)previously produced1 weekpartitions and now produces1 day(floor-driven), per the agreed default handling.