Skip to content

got XXXX parameters but the statement requires YY - potential BUG in 1.3.0 #690

Closed
@drsasa

Description

@drsasa

Hello,

Seems after upgrading to version 1.3.0 this do not work anymore:

// batch insert with structs

personStructs := []Person{
    {FirstName: "Ardie", LastName: "Savea", Email: "asavea@ab.co.nz"},
    {FirstName: "Sonny Bill", LastName: "Williams", Email: "sbw@ab.co.nz"},
    {FirstName: "Ngani", LastName: "Laumape", Email: "nlaumape@ab.co.nz"},
}

_, err = db.NamedExec(`INSERT INTO person (first_name, last_name, email)
    VALUES (:first_name, :last_name, :email)`, personStructs)

I on my queries getting:
pq: got XXXX parameters but the statement requires YY

but YY is not 65535 it is number of columns, in above example would be YY=3.

This is since >=1.3.0 version, earlier versions everything works.

Best regards,
Alexandar

Activity

ekrem95

ekrem95 commented on Jan 27, 2021

@ekrem95

Hello,

I have experienced a similar issue where if a query string ends with a white space or a new line the result of this code is:

	personStructs := []*Person{
		{FirstName: "Ardie", LastName: "Savea", Email: "asavea@ab.co.nz"},
		{FirstName: "Sonny Bill", LastName: "Williams", Email: "sbw@ab.co.nz"},
		{FirstName: "Ngani", LastName: "Laumape", Email: "nlaumape@ab.co.nz"},
	}

	_, err = tx.NamedExec(`INSERT INTO person (first_name, last_name, email)
		VALUES (:first_name, :last_name, :email) `, personStructs)

	fmt.Println(err)

pq: got 9 parameters but the statement requires 3

Version: 1.3.0, 1.3.1

Kind Regards

jmoiron

jmoiron commented on Jan 28, 2021

@jmoiron
Owner

If the issue is as @ekrem95 reports, I think this is fixed in the latest commit. Can you verify @drsasa, and if not, give me your specific use case?

drsasa

drsasa commented on Jan 29, 2021

@drsasa
Author

@jmoiron It was not space problem, but found what is.

if you have insert string like this:
"INSERT INTO xxx (aaa, bbb) VALUES (:ccc, :ddd)"
it will work.
But if you have like this:
"INSERT INTO xxx (aaa, bbb) VALUES (:ccc, :ddd);" <- here we have ; at end
will not. For now I changed and removed ; at end on all queries.

But since it is so common to end SQL queries with ; I think this is bug.

Kind regards,
Alexandar

stefpap

stefpap commented on Feb 15, 2021

@stefpap

I second what @drsasa said, removing the ; from the end of my query fixed the issue
Nice find, thanks :)

aarengee

aarengee commented on Mar 1, 2021

@aarengee

@jmoiron @hmgle
Facing a similar issue .
Removing ; at the end might solve it but is not reasonable as we might need to do more after batch insert like a batch upsert say using ON CONFLICT

See comment here.
#667 (comment)

added a commit that references this issue on May 11, 2021
cca8b15
dxps

dxps commented on Dec 4, 2021

@dxps

@drsasa , @ekrem95
I'm trying to look for a solution on a similar (not identical, ofc) case on my side. Could you please give some hints ❓

Using one of the standard Postgres' upsert approaches like this:

var addListingNamedSQL = "INSERT INTO listings (id, name, symbol) VALUES (:id, :name, :symbol) ON CONFLICT (id) DO UPDATE SET name = :name, symbol = :symbol"

(tried to have the SQL in one single line, instead of the multi line form using backticks, just to remove any other possible issue)

and then use it with NamedExec like this:

// TEST
listings = []domain.Listing{
   {Id: "1", Symbol: "1", Name: "1"},
   {Id: "2", Symbol: "2", Name: "2"},
}

r.NamedExec(addListingNamedSQL, listings)

it throws the error: pq: got 10 parameters but the statement requires 8

The classic INSERT works just fine:

INSERT INTO listings (id, name, symbol) VALUES (1, '1n', '1')
    ON CONFLICT (id) DO UPDATE SET name = '1n', symbol = '1';
drsasa

drsasa commented on Dec 4, 2021

@drsasa
Author

I think NamedExec do not support bulk UPSERT.
Just looked into code how handle this and what I found is,
that SQL expression is translated to:

INSERT INTO listings (id, name, symbol) VALUES ($1, $2, $3),($4, $5, $6) ON CONFLICT (id) DO UPDATE SET name = $7, symbol = $8

Which is not good interpretation of what you trying to do. So my conclusion is that UPSERT is not supported on batch insert.
And like you see this is reason for that error, here is expected 8 params and we sent 10

Cheers!

drsasa

drsasa commented on Dec 4, 2021

@drsasa
Author

I think NamedExec do not support bulk UPSERT. Just looked into code how handle this and what I found is, that SQL expression is translated to:

INSERT INTO listings (id, name, symbol) VALUES ($1, $2, $3),($4, $5, $6) ON CONFLICT (id) DO UPDATE SET name = $7, symbol = $8

Which is not good interpretation of what you trying to do. So my conclusion is that UPSERT is not supported on batch insert. And like you see this is reason for that error, here is expected 8 params and we sent 10

Cheers!

From other hand just saw that your PG syntax is not good :)
Your query should look like this:

INSERT INTO listings (id, name, symbol) VALUES (1, '1n', '1')
    ON CONFLICT (id) DO UPDATE SET name = excluded.name, symbol = excluded.symbol;

Which take us to this:

var addListingNamedSQL = "INSERT INTO listings (id, name, symbol) VALUES (:id, :name, :symbol) ON CONFLICT (id) DO UPDATE SET name = excluded.name, symbol = excluded.symbol"

And everything will work as expected. EXCLUDED contains rejected row so is common practice to use that instead values on CONFLICT part.

So really nothing wrong here on sqlx part.

Cheers!

dxps

dxps commented on Dec 4, 2021

@dxps

@drsasa Thanks for the detailed feedback, Alexandar!
Yeah, I didn't yet dive into the inners of sqlx to understand the error. So, thanks for the insight!

The SQL that I showed before it's just fine and it works without a problem.
I'd say that according to pg docs, excluded is that special table name used in this feature.
I could check further to make sure there are no side effects of not using it.

Meanwhile, to overcome this issue, I used the classical approach of doing all the "upserts" in a transaction. And the SQL used for this is:

var addListingSQL = `INSERT INTO listings (id, name, symbol) VALUES ($1, $2, $3) 
                     ON CONFLICT (id) DO UPDATE SET name = $2, symbol = $3`
drsasa

drsasa commented on Dec 4, 2021

@drsasa
Author

@dxps It is really safe to use EXCLUDED, I personally always use it. Like you saw in doc that is row which supposed to be inserted but was rejected because constraint violation. So its safe to say something = excluded.something, less binding params and less boilerplate code and on big batches I guess some performance improvement.

Cheers!

dxps

dxps commented on Dec 4, 2021

@dxps

@drsasa True! I'll update that SQL and start using excluded, at least to feel comfortable that I'm doing it by the book. 😊

Thanks again!
Cheers! 🙋‍♂️

2 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @jmoiron@aarengee@dxps@ekrem95@drsasa

        Issue actions

          got XXXX parameters but the statement requires YY - potential BUG in 1.3.0 · Issue #690 · jmoiron/sqlx