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

Deprecate passing parameters to Statement::execute*() #5556

Merged
merged 1 commit into from Jul 31, 2022

Conversation

morozov
Copy link
Member

@morozov morozov commented Jul 31, 2022

Q A
Type deprecation

Being able to bind parameters to the statements via execute($params) is a PDO legacy and has the following downsides:

  1. Unlike the Statement::bindParam() and ::bindValue() methods which accept 1-based numeric parameter positions, execute($params) expects 0-based indexes for positional parameters. This results in the inconsistency in logging statement parameters as reported in Inconsistent parameter indexes passed to the logging middleware #5525.
  2. No way to tell statically whether the $params in a given call to Statement::execute() should have integer or string keys since it depends on how parameters were declared in the SQL used to prepare the statement.
  3. The behavior of passing $params to execute() of a statement that already has parameters bound via bindParam() or bindValue() is undefined. Do parameters get merged or the previously bound parameters get ignored?
  4. The side effect of passing $params to execute() is undefined. Do the corresponding values remain bound to the statement or they are used only for this call? For reference, they remain bound to the wrapper statement but the driver-level behavior is undefined and is defined by the DBAL-level or the underlying driver.
  5. Not all underlying drivers support this API, so the DBAL has to emulate it by calling "bind" internally (e.g. for oci8).

The even bigger problem is that the wrapper level uses the same API for binding positional and named parameters that results in ugly and incorrect types like list<...>|array<string,...>. Whether the statement parameters are positional or named is decided at runtime. Deprecating this feature is the first step toward introducing separate APIs for positional and named parameters.

Affected API consumers:

  1. The ORM will not be affected since it minds the parameter types and uses Statement::bindValue() for binding parameters to the statement.
  2. The users of the DBAL will be barely affected since the wrapper-level API will still allow using the "shortcut" methods like executeQuery($sql, $params, $types).

@morozov morozov force-pushed the deprecate-statement-execute-params branch from 0c9f45e to 2c12361 Compare July 31, 2022 16:25
@morozov morozov marked this pull request as ready for review July 31, 2022 16:34
@mvorisek
Copy link
Contributor

Isn't this exactly the oposite of simillar newly introdue php function to mysqli - php/php-src#8660?

@morozov
Copy link
Member Author

morozov commented Jul 31, 2022

No. The newly introduced mysqli_execute_query() is analogous to Connection::executeQuery() and other "shortcut" methods that this deprecation doesn't affect.

@morozov morozov merged commit 4ccc03a into doctrine:3.4.x Jul 31, 2022
@morozov morozov deleted the deprecate-statement-execute-params branch July 31, 2022 21:49
@vinceAmstoutz
Copy link

In Symfony,executeQuery() is mentionned here : Querying with SQL.

I would like to confirm the change with you, please because a deprecation appears in my case since Symfony 6.3 was released.

Deprecation : User Deprecated: Passing $params to Statement::executeQuery() is deprecated. Bind parameters using Statement::bindParam() or Statement::bindValue() instead. (Statement.php:212 called by UtilisateurRepository.php:420, https://github.com/doctrine/dbal/pull/5556, package doctrine/dbal)

Tell my if I'm wrong, we can't do this right now :

// src/Repository/ProductRepository.php

// ...
class ProductRepository extends ServiceEntityRepository
{
    public function findProductByPriceQuantityAndCategory(int $price, float $quantity, string $category): array
    {
        $conn = $this->getEntityManager()->getConnection();

        $sql = '
            SELECT * FROM product p
            WHERE p.price > :price
            AND quantity >= :quantity
            AND category = :category   
            ORDER BY p.price ASC
            ';
        $stmt = $conn->prepare($sql);
        $resultSet = $stmt->executeQuery([
             'price' => $price,
             'quantity' => $quantity,
            'category ' => $category,
        ]);

        // returns an array of arrays (i.e. a raw data set)
        return $resultSet->fetchAllAssociative();
    }
}

Do we have to use bindParam() as many times as there's a parameter to pass, or is there another way, for example by passing an array of parameters somewhere?

   [
      'price' => $price,
      'quantity' => $quantity,
      'category ' => $category,
    ]

(I haven't found anything about this particular case on DBAL/Documentation/Data Retrieval And Manipulation and DBAL/UPGRADE).

@derrabus
Copy link
Member

derrabus commented Jun 1, 2023

There is an executeQuery() method on the connection that you can use instead.

@vinceAmstoutz
Copy link

There is an executeQuery() method on the connection that you can use instead.

Tanks 🙂@derrabus

OskarStark added a commit to symfony/symfony-docs that referenced this pull request Jun 19, 2023
…eQuery` (vinceAmstoutz)

This PR was submitted for the 6.3 branch but it was squashed and merged into the 6.2 branch instead.

Discussion
----------

Fix `doctrine/dbal` deprecation using `Statement::executeQuery`

Deprecation warning when `doctrine/dbal >= 3.4.*`.

In Symfony docs,executeQuery() is mentionned here : [Querying with SQL](https://symfony.com/doc/current/doctrine.html#querying-with-sql).

But since `3.4.*` passing parameters to `Statement::execute*()` functions is deprecated by `doctrine/dbal` with this warning:
`User Deprecated: Passing $params to Statement::executeQuery() is deprecated. Bind parameters using Statement::bindParam() or Statement::bindValue() instead. (Statement.php:212 called by User.php:40, doctrine/dbal#5556, package doctrine/dbal)`.

I've already ask in the [DBAL PR concerned](doctrine/dbal#5556 (comment)) how to use `executeQuery` from now on.

![image](https://github.com/symfony/symfony-docs/assets/46444652/e0b6938b-4f67-474a-a8f5-5f565b04e5b4)
[(in the same PR)](doctrine/dbal#5556 (comment))

So, after having tested it in a prod project, I propose an update of the doc in this sense.

Thanks again `@derrabus` 🙏

Commits
-------

5c8154b Fix `doctrine/dbal` deprecation using `Statement::executeQuery`
@fmonts
Copy link

fmonts commented Dec 8, 2023

IMHO this decision is a bad decision that doesn't provide any advantage to the community.
We are forced to change code in a way that adds verbosity for nothing, for example:

image

(of course we could use Connection::executeQuery(), but who is used to prepared statements finds it wrong to pass the same SQL string thousands of times in a loop)

I hope an alternative will be reintroduced.

@sofigio
Copy link

sofigio commented Dec 10, 2023

You can also call getWrappedConnection() and do that with PDO, but yes it was a nice feature to have

@Evgeny1973
Copy link

Evgeny1973 commented Feb 28, 2024

What if there is a variable number of values? ($values - array with variable number of values).
$in = implode(',', array_fill(0, count($values), '?'));
Now in query
$sql = SELECT COUNT(*) AS count FROM table t <...> AND t.id IN ({$in});
$stmt = $this->connection->prepare($sql);
return $stmt->executeQuery($values)->fetchOne();
How can I do data substitution now?

@derrabus
Copy link
Member

derrabus commented Feb 28, 2024

$sql = 'SELECT COUNT(*) AS count FROM table t <...> AND t.id IN (:ids)';

return $this->connection->executeQuery($sql, ['ids' => $values], ['ids' => ArrayParameterType::INTEGER]);

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

Successfully merging this pull request may close these issues.

None yet

8 participants