Skip to content

Calling model#id is broken for Rails 7.2+ for composite primary keys #89

@andrepiske

Description

@andrepiske

Hello!
I found the #id method is broken in some scenarios when a model has include PgParty::Model. The way it breaks is that model_instance.id always returns nil. The scenario seems to be when the partitioned table has a composite primary key AND rails version is >= 7.2 (in my experiments, it breaks in 7.2 and 8.0 but works fine in 7.1).

A partitioned table like the one below will have this issue:

CREATE TABLE bigint_timestamp_ranges (
  id BIGSERIAL NOT NULL,
  updated_at TIMESTAMP WITHOUT TIME ZONE NOT NULL,
  created_at TIMESTAMP WITHOUT TIME ZONE NOT NULL,
  PRIMARY KEY (id, created_at)
) PARTITION BY RANGE (created_at);

CREATE TABLE bigint_timestamp_ranges_a
  PARTITION OF bigint_timestamp_ranges
  FOR VALUES FROM ('2024-12-19T00:00:00Z') TO ('2024-12-20T00:00:00Z');

CREATE TABLE bigint_timestamp_ranges_b
  PARTITION OF bigint_timestamp_ranges
  FOR VALUES FROM ('2024-12-20T00:00:00Z') TO ('2024-12-21T00:00:00Z');

(these exact partitions aren't important but I've included them for the sake of completeness)

I'll try to show the issue by having two PRs opened in a fork of pg_party off 1.9.0.

One PR is this one: andrepiske#1 -- I've added a BigintTimestampRange model and I'm creating its table a little differently than other specs. In the spec/dummy/db/schema.rb file, I'm dropping and re-creating indexes so that the table becomes just like the sql above. In this PR, the issue is fixed and the fix is in the lib/pg_party/model/shared_methods.rb file, by using self.primary_key = ... instead of @primary_key = .... I made a spec out of spec/integration/model/bigint_date_range_spec.rb (mostly copied and adjusted) to show the issue and the github action runs that spec file only. With the fix in, the spec passes: https://github.com/andrepiske/pg_party/actions/runs/12397946007

Then here is another PR removing the fix (it stems from that first PR above): andrepiske#2 -- the broken code is what's in pg_party main branch now. Here is a github action running it and failing for rails >= 7.2 but passing for 7.1: https://github.com/andrepiske/pg_party/actions/runs/12397977681

I opened a draft pull request with the fix in #88 but when I went for writing a spec for it, I wasn't sure really how to. It seems pg_party doesn't oficially support composite primary keys, so it'd be a larger overhaul of the gem to make it work, no?

It also seems that what introduced this issue is rails/rails#49808.

@marcoroth pinging you in case you have interest in this one, given your discussion in the PR above and your contribution in #83 -- which hopefully I'm not breaking with the changes I'm proposing here.


Sidenote: For what it's worth, I have a table pretty much like that in a gem I'm working on at https://github.com/trusted/iron_trail/blob/3b3fff4c/lib/generators/iron_trail/templates/create_irontrail_changes.rb.erb#L3-L17

I'm now thinking whether or not I should have that composite primary key at all. The reason it's composite is because postgres requires the partitioned column to be part of the index. Another option would be to have the primary key in the partitions only, that'd allow me to have a non-composite primary key and it'd make things easier -- especially since I'm not worried about duplicate ids given its values are generated from a sequence.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions