bindInsert doesn't support multiple autoincrement columns

[siddharth178]

I am on PostgreSQL, and have a table with 2 auto increment columns along with other columns. One of those auto increment columns is PK.

What I was expecting is, calling SetKeys on PK and then setting autoincrement tag on 2nd auto increment column should make it work properly - i.e. insert should insert nextvals in both those columns and object that I am inserting should get those values reflected. But it looks like

gorp/table_bindings.go

Line 160 in f3677d4

if plan.autoIncrIdx > -1 {

doesn't handle multiple autoincrement columns. It only generates returning clause with latest autoincrement column.

Any specific reason to not include all autoincrement columns as part of returning clause?

[nelsam]

This is probably an artifact of trying to support a bunch of databases, many of which don't conform well to the SQL standard. For example: MySQL does not have a returning clause.

If you would like to make a PR to fix it, you're welcome to!

[siddharth178]

What I am thinking of to fix this is

• Main idea here is to get autoincrement struct field tag working when there is another column set as PK.

• Change Dialect's AutoIncrInsertSuffix method to accept slice of column names

• Other than Postgresql implementation all other implementations will still return "" - empty string

  • Postgres implementation will return returning clause with all the column names that are passed in as argument

• Change TableMap.bindInsert to keep track of all the column names in a separate string slice for which autoincrement is needed/requested

  • Then pass that string slice to AutoIncrInsertSuffix

[siddharth178]

Looking more into the code I realised that above approach will only modify the query to return multiple values. But I will also need to update insert and handle cases like TargetQueryInserter, TargetedAutoIncrInserter, IntegerAutoIncrInserter etc. which are currently written to handle single autoincrement field.