Skip to content
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

table with sequence id from another table : the generated chronomodel_XYZ_insert() is broken #91

Closed
thomas-peyric opened this issue Sep 19, 2019 · 2 comments

Comments

@thomas-peyric
Copy link

Hi,

first i use ruby 2.3 / rails 4.2 with chronomodel 0.9.1
and postgres 9.4.12

when a change a table (with an id sequence of anothe table) to a chronomodel table using rails migration, the generated chronomodel_XYZ_insert() is broken

Here a table with is own id sequence (no pb on this table)

my_db=> \d temporal.table_a
                                                    Table "public.table_a"
                Column                |           Type           |                            Modifiers
--------------------------------------+--------------------------+-----------------------------------------------------------------
 id                                   | integer                  | not null default nextval('table_a_id_seq'::regclass)
 label                                | character varying        |
Indexes:
    "table_a_pkey" PRIMARY KEY, btree (id)

I want to apply chronomoel to table_a, so a do migration like this :

change_table :table_a, temporal: true, copy_data: true

here the tables informations :


my_db=> \d temporal.table_a
                                                    Table "temporal.table_a"
                Column                |           Type           |                            Modifiers
--------------------------------------+--------------------------+-----------------------------------------------------------------
 id                                   | integer                  | not null default nextval('temporal.table_a_id_seq'::regclass)
 label                                | character varying        |
Indexes:
    "table_a_pkey" PRIMARY KEY, btree (id)


my_db=> \d history.table_a
                                                      Table "history.table_a"
                Column                |            Type           |                            Modifiers
--------------------------------------+---------------------------+--------------------------------------------------------------
 id                                   | integer                   | not null default nextval('temporal.table_a_id_seq'::regclass)
 label                                | character varying         |
 hid                                  | integer                   | not null default nextval('history.table_a_hid_seq'::regclass)
 validity                             | tsrange                   | not null
 recorded_at                          | timestamp without time zone | not null default timezone('UTC'::text, now())
Indexes:
    "table_a_pkey" PRIMARY KEY, btree (hid)
    "index_table_a_temporal_on_lower_validity" btree (lower(validity))
    "index_table_a_temporal_on_upper_validity" btree (upper(validity))
    "index_table_a_temporal_on_validity" gist (validity)
    "table_a_inherit_pkey" btree (id)
    "table_a_instance_history" btree (id, recorded_at)
    "table_a_recorded_at" btree (recorded_at)
    "table_a_timeline_consistency" EXCLUDE USING gist (id WITH =, validity WITH &&)
Inherits: temporal.table_a


my_db=> \d public.table_a
                    View "public.table_a"
        Column         |           Type           |   Modifiers
-----------------------+--------------------------+---------------
 id                    | integer                  |
 label                 | character varying        |
Triggers:
    chronomodel_delete INSTEAD OF DELETE ON table_a FOR EACH ROW EXECUTE PROCEDURE chronomodel_table_a_delete()
    chronomodel_insert INSTEAD OF INSERT ON table_a FOR EACH ROW EXECUTE PROCEDURE chronomodel_table_a_insert()
    chronomodel_update INSTEAD OF UPDATE ON table_a FOR EACH ROW EXECUTE PROCEDURE chronomodel_table_a_update()


======================================================================


--
-- Name: chronomodel_table_a_insert(); Type: FUNCTION; Schema: public; Owner: -
--

CREATE FUNCTION chronomodel_table_a_insert() RETURNS trigger
LANGUAGE plpgsql
AS $$
    BEGIN
      IF NEW.id IS NULL THEN
        NEW.id := nextval('temporal.table_a_id_seq');
      END IF;

      INSERT INTO temporal.table_a ( id, "label" )
      VALUES ( NEW.id, NEW."label" );

      INSERT INTO history.table_a ( id, "label", validity )
      VALUES ( NEW.id, NEW."label", tsrange(timezone('UTC', now()), NULL) );

      RETURN NEW;
    END;
  $$;

now i have a second table 'table_b' which must have the same sequence than 'table_a'

my_db=> \d temporal.table_b
                                                    Table "public.table_b"
                Column                |           Type           |                            Modifiers
--------------------------------------+--------------------------+-----------------------------------------------------------------
 id                                   | integer                  | not null default nextval('temporal.table_a_id_seq'::regclass)
 label                                | character varying        |
Indexes:
    "table_b_pkey" PRIMARY KEY, btree (id)

I want to apply chronomodel to table_b, so a do migration like this :

change_table :table_b, temporal: true, copy_data: true

here the tables informations :

my_db=> \d temporal.table_b
                                                    Table "temporal.table_b"
                Column                |           Type           |                            Modifiers
--------------------------------------+--------------------------+-----------------------------------------------------------------
 id                                   | integer                  | not null default nextval('temporal.table_a_id_seq'::regclass)
 label                                | character varying        |
Indexes:
    "table_b_pkey" PRIMARY KEY, btree (id)


my_db=> \d history.table_b
                                                      Table "history.table_b"
                Column                |            Type           |                            Modifiers
--------------------------------------+---------------------------+--------------------------------------------------------------
 id                                   | integer                   | not null default nextval('temporal.table_a_id_seq'::regclass)
 label                                | character varying         |
 hid                                  | integer                   | not null default nextval('history.table_b_hid_seq'::regclass)
 validity                             | tsrange                   | not null
 recorded_at                          | timestamp without time zone | not null default timezone('UTC'::text, now())
Indexes:
    "table_b_pkey" PRIMARY KEY, btree (hid)
    "index_table_b_temporal_on_lower_validity" btree (lower(validity))
    "index_table_b_temporal_on_upper_validity" btree (upper(validity))
    "index_table_b_temporal_on_validity" gist (validity)
    "table_b_inherit_pkey" btree (id)
    "table_b_instance_history" btree (id, recorded_at)
    "table_b_recorded_at" btree (recorded_at)
    "table_b_timeline_consistency" EXCLUDE USING gist (id WITH =, validity WITH &&)
Inherits: temporal.table_b


my_db=> \d public.table_b
                    View "public.table_b"
        Column         |           Type           |   Modifiers
-----------------------+--------------------------+---------------
 id                    | integer                  |
 label                 | character varying        |
Triggers:
    chronomodel_delete INSTEAD OF DELETE ON table_b FOR EACH ROW EXECUTE PROCEDURE chronomodel_table_b_delete()
    chronomodel_insert INSTEAD OF INSERT ON table_b FOR EACH ROW EXECUTE PROCEDURE chronomodel_table_b_insert()
    chronomodel_update INSTEAD OF UPDATE ON table_b FOR EACH ROW EXECUTE PROCEDURE chronomodel_table_b_update()


======================================================================


--
-- Name: chronomodel_table_b_insert(); Type: FUNCTION; Schema: public; Owner: -
--

CREATE FUNCTION chronomodel_table_b_insert() RETURNS trigger
LANGUAGE plpgsql
AS $$
    BEGIN
      IF NEW.id IS NULL THEN
        NEW.id := nextval('');      -- <== THE ERROR 
      END IF;

      INSERT INTO temporal.table_b ( id, "label" )
      VALUES ( NEW.id, NEW."label" );

      INSERT INTO history.table_b ( id, "label", validity )
      VALUES ( NEW.id, NEW."label", tsrange(timezone('UTC', now()), NULL) );

      RETURN NEW;
    END;
  $$;

in summary :

Table "temporal.table_a"

  • id ==> not null default nextval('temporal.table_a_id_seq'::regclass)

Table "history.table_a"

  • id ==> not null default nextval('temporal.table_a_id_seq'::regclass)
  • hid ==> not null default nextval('history.table_a_hid_seq'::regclass)

chronomodel_table_a_insert()

  • NEW.id := nextval('temporal.table_a_id_seq');

Table "temporal.table_b"

  • id ==> not null default nextval('temporal.table_a_id_seq'::regclass)

Table "history.table_b"

  • id ==> not null default nextval('temporal.table_a_id_seq'::regclass)
  • hid ==> not null default nextval('history.table_b_hid_seq'::regclass)

chronomodel_table_b_insert()

  • NEW.id := nextval(''); -- <== THE ERROR

==>

as you can see the nextval in the chronomodel_table_b_insert is broken because there is no sequence
this problem only occured when i use an id sequence uses by other table ( same things if table_a is or is not a chronomodel table)

I know i use an old version of chronomodel ... but can you take a look on this.
maybe i forgot an option in migration, or maybe it's a bug ...

thanks a lot .

Thomas

@vjt
Copy link
Contributor

vjt commented Oct 1, 2019

This is expected AR/Postgres behaviour, because the sequence used in table_b is owned by the id column in table_a. I recreated the situation with:

chronomodel=# create table temporal.table_a (id serial primary key, label varchar);
CREATE TABLE
chronomodel=# \d temporal.table_a
                                     Table "temporal.table_a"
 Column |       Type        | Collation | Nullable |                   Default
--------+-------------------+-----------+----------+----------------------------------------------
 id     | integer           |           | not null | nextval('temporal.table_a_id_seq'::regclass)
 label  | character varying |           |          |
Indexes:
    "table_a_pkey" PRIMARY KEY, btree (id)
chronomodel=# create table temporal.table_b (id integer primary key default nextval('temporal.table_a_id_seq'::regclass), label varchar);
CREATE TABLE
chronomodel=# \d temporal.table_b
                                     Table "temporal.table_b"
 Column |       Type        | Collation | Nullable |                   Default
--------+-------------------+-----------+----------+----------------------------------------------
 id     | integer           |           | not null | nextval('temporal.table_a_id_seq'::regclass)
 label  | character varying |           |          |
Indexes:
    "table_b_pkey" PRIMARY KEY, btree (id)

ChronoModel uses the serial_sequence method from the PostgreSQLAdapter when creating the INSERT trigger, that indeed returns nil on table_b in this case:

[5] pry(main)> ActiveRecord::Base.connection.serial_sequence 'temporal.table_a', 'id'
=> "temporal.table_a_id_seq"
[6] pry(main)> ActiveRecord::Base.connection.serial_sequence 'temporal.table_b', 'id'
=> nil

The AR method internally calls pg_get_serial_sequence(), whose documentation says The function probably should have been called pg_get_owned_sequence; its current name reflects the fact that it's typically used with serial or bigserial columns.

Each sequence has an owner column and pg_get_serial_sequence returns that owner, and in our case the id of table_b is not owned by any sequence!

chronomodel=# create sequence temporal.foobar increment by 1 owned by none;
CREATE SEQUENCE
chronomodel=# create table temporal.table_a (id integer primary key default nextval('temporal.foobar'::regclass), label varchar);
CREATE TABLE

produces in Ruby:

[19] pry(main)> adapter.serial_sequence 'temporal.table_a', 'id'
=> nil

However, I found that AR has a pk_and_sequence_for method that suits our purposes:

custom shared sequence

[31] pry(main)> adapter.pk_and_sequence_for('temporal.table_a').last.to_s
=> "temporal.foobar"
[32] pry(main)> adapter.pk_and_sequence_for('temporal.table_b').last.to_s
=> "temporal.foobar"

sequence created as a consequence of a serial column

[33] pry(main)> adapter.pk_and_sequence_for('temporal.table_a').last.to_s
=> "temporal.table_a_id_seq"
[35] pry(main)> adapter.pk_and_sequence_for('temporal.table_b').last.to_s
=> "temporal.table_a_id_seq"

So I am now giving it a try.

@thomas-peyric
Copy link
Author

Thanks a lot,
I will test ASAP ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants