#98 ✓resolved
Bjørn Arild Mæland

Better INSERT queries for Postgres

Reported by Bjørn Arild Mæland | December 14th, 2007 @ 10:06 AM

Current behaviour:

INSERT INTO () VALUES (instance->values)

SELECT lastval();

set the id to lastval();

Should change to:

INSERT RETURNING id;

For 8.2, and:

id = SELECT nextval(sequence_name);

INSERT INTO (id, column1, column2, column3, etc.) VALUES

(id, value1, value2, value3);

For anything older than 8.2.

Comments and changes to this ticket

  • Sam Smoot

    Sam Smoot December 27th, 2007 @ 10:26 AM

    • Assigned user set to “Yehuda Katz”
    • State changed from “new” to “open”

    Is the second syntax really what we want? What if some constraint kills the insert? Then we have an invalid id assigned...

    We also need to be sure whatever we do works with natural keys as well. Probably easy enough, but just something to keep in mind. We don't always want to issue the RETURNING clause.

    Yehuda Katz had some concerns with using RETURNING before, but I'm not sure I remember the conversation exactly so...

  • Yehuda Katz

    Yehuda Katz December 28th, 2007 @ 12:57 AM

    The issue I had with RETURNING is that it'll require us to munge up queries if people use raw queries with DM.

  • Sam Smoot

    Sam Smoot December 28th, 2007 @ 09:55 PM

    • Assigned user changed from “Yehuda Katz” to “Bjørn Arild Mæland”

    Ah... right. Thanks Yehuda.

    So yeah... because RETURNING would be pretty much impossible to use with Result#last_insert_id, it needs to stay out of DO.

    That doesn't mean we can't special case inserts with serial keys in the PostgresAdapter itself, but that's a task I'll leave to someone else. ;-) Shouldn't be too hard really...

    if table.keys.size == 1 && table.keys.first.serial?
      "INSERT INTO #{table.to_sql} RETURNING #{table.keys.first.to_sql}  (...) VALUES (...)"
    else
      "INSERT INTO #{table.to_sql} (...) VALUES (...)"
    end
    
  • Dan Kubb (dkubb)
  • Sam Smoot

    Sam Smoot April 27th, 2008 @ 10:09 PM

    • State changed from “open” to “resolved”

    Ok this is done and works with RETURNING. The backwards compat portion doesn't, but I'll let someone open a new ticket on that portion if we really need it...

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

People watching this ticket

Tags

Pages