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

Fix RegEx for multi-line inserts #667

Merged
merged 1 commit into from
Jan 24, 2021
Merged

Fix RegEx for multi-line inserts #667

merged 1 commit into from
Jan 24, 2021

Conversation

romandvoskin
Copy link
Contributor

@romandvoskin romandvoskin commented Nov 18, 2020

Fixes issues with multi line inserts. #648, #640, #622

@romandvoskin romandvoskin changed the title Fix rex and add now to tests Fix RegEx for multi-line inserts Nov 18, 2020
@coveralls
Copy link

coveralls commented Nov 18, 2020

Pull Request Test Coverage Report for Build 160

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+4.9%) to 73.304%

Totals Coverage Status
Change from base Build 155: 4.9%
Covered Lines: 1178
Relevant Lines: 1607

💛 - Coveralls

@romandvoskin
Copy link
Contributor Author

@jmoiron Please take a look. The fix is in the RegEx. now is added to test harness to encourage sql function testing.

@jmoiron jmoiron merged commit 7b5a3fc into jmoiron:master Jan 24, 2021
@jmoiron
Copy link
Owner

jmoiron commented Jan 24, 2021

fixes #648, #640, #622

@aarengee
Copy link

aarengee commented Mar 1, 2021

@jmoiron This regex assumes the value_list to be inserted is at the end of the list and has reopened a previous issue #505

This is my query

INSERT INTO table(col1, col2, col3, eligible ,created_at, updated_at)
 					VALUES (:col1, :col2, :col3 :eligible ,now(), now()) 
 					ON CONFLICT (col1, col2)
 					DO UPDATE SET col2 = excluded.col2,eligible = excluded.eligible, updated_at = now() RETURNING *

When I try to insert a struct[] with 2 elements in it I get

pq: got 8 parameters but the statement requires 4

I am using potsgres and sqlx@1.3.0 with pq@1.2.0

@aarengee
Copy link

aarengee commented Mar 3, 2021

For anyone looking for a solution I was able to make use of inner functions and make a bulk upsert work

	query, queryArgs, _ := db.BindNamed(queryWithoutONCONFLICTWhichEndsInBracket, yourSlice)
	query = db.Rebind(queryWithoutONCONFLICTWhichEndsInBracket)
	query = queryWithoutONCONFLICTWhichEndsInBracket + onConflictStatement
	rows, _ := repo.db.Queryx(query, queryArgs...)

This should work .

cc : @matthisk

@smoussa
Copy link

smoussa commented Mar 9, 2021

@aarengee Unfortunately that doesn't work for me. I get pq: syntax error at or near ":". Here is the only place I use the colon syntax:

... VALUES (:col1, :col2, ...) ... ON CONFLICT

@aarengee
Copy link

aarengee commented Mar 13, 2021

Hey @smoussa Let me give a Complete Example

	sliceOfStructs := []model.Gamer{
		{
		UserID:   "123",
		Name: "aarengee",
		Address: "xd",
		Eligible: true,
		},
		{
			UserID:   "1234",
			Name: "aarengeeAgain",
			Address: "xd",
			Eligible: false,
		},
	}
	upsertQuery := "INSERT INTO gamer_details (user_id, name, address, eligible,updated_at) VALUES (:user_id,:name,:address,:eligible,now())"
	onConflictStatement := " ON CONFLICT (user_id, name) DO UPDATE SET address = excluded.address,eligible = excluded.eligible, updated_at = now() RETURNING *"
	query, queryArgs, _ := db.BindNamed(upsertQuery, sliceOfStructs)
	query = db.Rebind(query)
	query = query + onConflictStatement
	rows, err := db.Queryx(query, queryArgs...)

As stated I am using potsgres and sqlx@1.3.0 with pq@1.2.0. Although DB flavor and pq shouldnt matter. Hope this helps.
Maybe you can assist with a somewhat working code and we can work.

@smoussa
Copy link

smoussa commented Apr 6, 2021

@aarengee Thanks for this. You'll notice that you've used the bound query string in db.Rebind(query), whereas in your first example you've used db.Rebind(queryWithoutONCONFLICTWhichEndsInBracket).

Your example has fixed the colon issue, but I've run into another problem when upserting two struct objects:

pq: got 16 parameters but the statement requires 8

This is the case when bulk upserting more than one item. In your example with two struct objects you would get:

pq: got 8 parameters but the statement requires 4

@smoussa
Copy link

smoussa commented Apr 6, 2021

Using @abraithwaite's PR #718, I've managed to find a solution. The implementation is also simpler:

query := "INSERT INTO ... VALUES ... ON CONFLICT ... DO UPDATE SET ... RETURNING ..."
rows, err := db.NamedQuery(query, sliceOfStructs)

Thanks @aarengee nonetheless! Hope this helps.

@aarengee
Copy link

aarengee commented Apr 6, 2021

This is the case when bulk upserting more than one item. In your example with two struct objects you would get:

pq: got 8 parameters but the statement requires 4

Hey @smoussa , If you see in my example I am indeed bulk upserting with two structs. I was able to succesfully do the bulk upsert with two structs in my codebase.

P.S. I just deployed this code to PROD like 2 hours ago and its working just fine. As you know I can only use stable releases in a production ready code base .

Using @abraithwaite's PR #718, I've managed to find a solution. The implementation is also simpler:

Yes, This is indeed the right way and how @jmoiron intended it to be used. This was fixed then broken and now has been fixed again. 🤞 .

Hopefully this makes its way into a stable version release and I can use the same instead of the hack I managed to pull together.

@squillace91
Copy link

I kept still getting the error when doing a batch insert with ON CONFLICT, so I found a solution that worked for me with a help of a friend. This was very hard to find but you can see more info here. https://www.prisma.io/dataguide/postgresql/inserting-and-modifying-data/insert-on-conflict#using-the-do-update-action

DO UPDATE SET (var1, var2, var3, var4) = (excluded.var1, excluded.var2, excluded.var3, excluded.var4)

Hope this helps!

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

Successfully merging this pull request may close these issues.

6 participants