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

SQL parser issue for a specific combination of replacements, "?" symbol matching problem. #17067

Open
3 of 6 tasks
Stashevich opened this issue Feb 8, 2024 · 3 comments
Open
3 of 6 tasks

Comments

@Stashevich
Copy link

Stashevich commented Feb 8, 2024

Issue Creation Checklist

  • I understand that my issue will be automatically closed if I don't fill in the requested information
  • I have read the contribution guidelines

Bug Description

When creating a raw query, the parser can run into a situation when it skips a part of a query, which supposed to be replaced by values from REPLACEMENTS array.
Lets assume that a user trying to implement a seed like below:

Reproducible Example

   module.exports = {
    up: (q) =>
      q.sequelize.transaction(async (transaction) => {
        await q.sequelize.query(
          "CREATE USER IF NOT EXISTS ?@? IDENTIFIED BY ?", {
            raw: true,
            replacements: [ "test", "localhost", "abc123" ],
            transaction
          });
  
        await q.sequelize.query("FLUSH PRIVILEGES", { raw: true, transaction });
      }),
  
    down: (q) =>
      q.sequelize.query(
        "DROP USER IF EXISTS ?@?", {
          raw: true,
          replacements: [ "test", "localhost" ],
        }
      )
  };

Generated in such case SQL looks next:

 CREATE USER IF NOT EXISTS 'test'@? IDENTIFIED BY 'localhost';

What do you expect to happen?

Additional check should be added to parser to handle replacement constructions like this: "?@?".

What is actually happening?

Parser skips an occurrence of the second '?' in "?@?" and therefore inserts items of replacements array in a wrong order.

Environment

  • Sequelize version: sequelize@6.33.0

  • Node.js version: v18.18.0

  • Database & Version: mysql:8.0

  • Connector library & Version: mysql2@3.6.1

  • Issue file location: node_modules/sequelize/lib/utils/sql.js

  • Line number: starts at 139:

  • Code block snippet:

    ...
    if (isPositionalReplacements && char === "?") {
      const previousChar = sqlString[i - 1];
      if (!canPrecedeNewToken(previousChar) && previousChar !== "[") {
        continue;
      }
      ...
    }
   ...

Would you be willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time and I know how to start.
  • Yes, I have the time but I will need guidance.
  • No, I don't have the time, but my company or I are supporting Sequelize through donations on OpenCollective.
  • No, I don't have the time, and I understand that I will need to wait until someone from the community or maintainers is interested in resolving my issue.

Indicate your interest in the resolution of this issue by adding the 👍 reaction. Comments such as "+1" will be removed.

@Stashevich Stashevich added pending-approval Bug reports that have not been verified yet, or feature requests that have not been accepted yet type: bug labels Feb 8, 2024
@ephys
Copy link
Member

ephys commented Feb 8, 2024

Here is a thread about a previous conversation related to this issue: #15410

@ephys ephys removed the pending-approval Bug reports that have not been verified yet, or feature requests that have not been accepted yet label Feb 8, 2024
@Stashevich
Copy link
Author

Here is a thread about a previous conversation related to this issue: #15410

Thanks for the quick update. I see, so they are both about the same thing. Then, should I close this one with "duplicates" comment?

@ephys
Copy link
Member

ephys commented Feb 10, 2024

The other one is a discussion that was never turned into a bug report so I'm keeping yours open so we don't lose track of it again :)

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

No branches or pull requests

2 participants