0

I have an REST-ish endpoint that creates a day object and sets its order property to whatever the maximum order is +1. I'm having an issue where calling that endpoint in rapid succession results in some of the days having the same order. How do I solve this?

SQL Query is like so.

insert into "days" ("order", "program_id") values (
    (select max(days.order)+1
    from "days"
    where "days"."program_id" = '5'), '5')
    returning *

And it results in something like

{"program_id":5,"id":147,"order":38}

{"program_id":5,"id":150,"order":38}

{"program_id":5,"id":148,"order":38}

{"program_id":5,"id":149,"order":38}

{"program_id":5,"id":151,"order":39}

{"program_id":5,"id":152,"order":40}

{"program_id":5,"id":153,"order":41}

If it helps, I'm on Node (Express) and using Knex and Objection to build my queries for a Postgres database. The JavaScript code is as follows.

json.order = knex.select(knex.raw('max(days.order)+1'))
                .from('days')
                .where('days.program_id', json.program_id);

return await Days
             .query(trx)
             .returning('*')
             .insert(json);

I'm also using max+1 as I want the order values to increment on a per program basis. So days of a program will have unique orders, but it is possible to have days of different programs with the same order.

Thanks!

Community
  • 1
  • 1
n a
  • 301
  • 2
  • 10
  • 2
    That is a really bad idea. For one because it won't scale with growing tables but more importantly it simply won't work correctly with concurrent transactions. Use a a `serial` or `identity` column instead –  Nov 19 '18 at 06:44
  • 2
    Building "significance" into an identity/primary key is not good practice. You can always calculate the number of orders per program, and/or you can include the program id or name on an order if needed. – Paul Maxwell Nov 19 '18 at 07:09
  • Moving away from max makes sense, but that raises another issue. Every `program` has a `start_date`, and that `start_date` is changeable by the user. I'm using the `order` property of a `day` to calculate its `date` from the `program` `start_date`. Knowing the actual `date` of a `day` allows me to query for all `days` that are in the past, all `days` that are today, and all `days` that are in the future. – n a Nov 19 '18 at 07:25
  • I could have a `date` property on each `day` and when the `program` `start_date` changes, I have to recalculate all the subsequent `date` properties of every single `day` associated with the `program`. Hmm. Thoughts @a_horse_with_no_name @Used_By_Already ? – n a Nov 19 '18 at 07:27

1 Answers1

0

You could probably add select ... for update locking to the subquery where you are calculating max+1:

json.order = knex.select(knex.raw('max(days.order)+1'))
                .forUpdate() // <-------- lock rows 
                .from('days')
                .where('days.program_id', json.program_id);

So after that no other concurrent connection can read those rows that are used for calculating max until this transaction ends.

Mikael Lepistö
  • 18,909
  • 3
  • 68
  • 70
  • 2
    you can't use `select .. for update` with an aggregate function in Postgres –  Nov 19 '18 at 07:35
  • @ a_horse_with_no_name Thanks, I didn't knew that. I thought it would just lock all the rows that were used to aggregate the result. In that case one has to do one extra select for the same rows without aggregating them or use explicit advisory locks. – Mikael Lepistö Nov 19 '18 at 14:58