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

Can execute multiple statements in a single query #601

Open
alxndrsn opened this issue May 10, 2024 · 4 comments
Open

Can execute multiple statements in a single query #601

alxndrsn opened this issue May 10, 2024 · 4 comments
Labels

Comments

@alxndrsn
Copy link
Contributor

Expected Behavior

Attempting to run multiple statements should throw an Error, and the statement should not be executed.

Current Behavior

Attempting to run multiple statements should throw an Error, and the statements are executed.

Possible Solution

I think a fix would require a change in node-postgres to force use of extended queries when there is no prepared statement or query parameters.

Steps to Reproduce

  test('allows multiple statements in a single query, but throws error', async (t) => {
    const pool = await createPool(t.context.dsn, {
      driverFactory,
    });

    const result1 = await pool.anyFirst(sql.unsafe`
      SELECT name FROM person
    `);

    t.deepEqual(result1, []);

    const error = await t.throwsAsync(
      pool.query(sql.unsafe`
        INSERT INTO person (name) VALUES('alice');
        INSERT INTO person (name) VALUES('bob');
      `),
    );

    t.true(error instanceof InvalidInputError);

    const result = await pool.anyFirst(sql.unsafe`
      SELECT name FROM person
    `);

    t.deepEqual(result, ['alice', 'bob']);
  });

Logs

Passing test can be seen at:

@alxndrsn alxndrsn added the bug label May 10, 2024
@alxndrsn
Copy link
Contributor Author

@gajus
Copy link
Owner

gajus commented May 13, 2024

Unfortunately, the only way to support this at Slonik-level would be if we had a SQL tokenizer.

I long wanted SQL tokenizer for other use cases, but there aren't any that have good enough coverage for production adoption.

@gajus
Copy link
Owner

gajus commented May 14, 2024

@alxndrsn another option to prevent this at Slonik level (assuming brianc/node-postgres#3214 does not get merged), would be to auto generate statement name based on the query. The overhead would be pretty minimal and it would achieve more or less the same effect as the proposed pg patch.

@alxndrsn
Copy link
Contributor Author

@alxndrsn another option to prevent this at Slonik level (assuming brianc/node-postgres#3214 does not get merged), would be to auto generate statement name based on the query.

👍 but I suspect there is some extra overhead to named prepared statements vs unnamed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants