getUnquotedStatementFragments no longer works for very long query since 2.8.1

[fletanoux]

BC Break Report

Q	A
BC Break	yes
Version	2.8.1

Summary

Function getUnquotedStatementFragments no longer works properly if the sql (postgresql) request is too long/complex. As a result params passed to executeQuery/executeUpdate are no longer used and we are getting this error :

"SQLSTATE[08P01]: <<Unknown error>>: 7 ERROR: bind message supplies 0 parameters, but prepared statement "" requires 2"

Previous behaviour

You could have an extremy long and complex query with several json_build_object imbricated into each other, and params would get replaced as intended.

Current behavior

WIth the exact same request, params are no longer replaced, and we get an error instead.

How to reproduce

Just make a very long query, with a lot of fields (it seems to bug above 40 fields) and some parameters to replace. Pass that query to a executeUpdate/Query method, and wait for the error to come. In my case the sql query is mostly composed of json_build_object imbricated in each other.

I've traced back the error to the method getUnquotedStatementFragments in the file SQLParsertUtils.php which seemed have been modified in the following commit :

0a7d5f0#diff-e7dd540cfb86c74ce7361e2bd00053cf

I've run the same test on the current version of DBAL so 2.9.2 but with the old code from 2.8.0 for the method getUnquotedStatementFragments and it works, so the problem is 100% there.

I've also found out that by increasing the PCRE's backtracking limit ( ini_set('pcre.backtrack_limit', 10000000);
) it also works with current code. (

[SenseException]

The bugfix of #3277 probably changed recursion/backtrack with the pattern. What's the size of your statement, that is injected into the mentioned method?

[fletanoux]

The statement i used to do my test with is just over 15k characters long.

[duvall]

Same problem here with a query of 17748 characters, I set up ini_set('pcre.backtrack_limit', 10000000); and it works just fine

[SenseException]

A query this long is probably an edge case and it can be averted by the change of the pcre config value.

Despite that, I would like to suggest that a PR with a test should be added for the specific method on default config values to prevent future behaviours like described when new changes are introduced. For an Regex, it's not only the length, but also the pattern that decides about how it get parsed with a number of recursions, so the suggested test would be more a "rule of thumb".

If the Regex can be changed without breaking the fix of #3277, this mentioned test would be needed too. PRs are welcome.

[morozov]

There's code with a similar purpose in OCI8Statement::convertPositionalToNamedPlaceholders() which uses much more simple regular expressions but may have poorer performance. I'm curious if it makes sense to replace them with a real parser since the syntax is fairly simple.

/cc @Majkl578

[morozov]

Closing as irrelevant as of #4397. If the issue is still reproducible, please file a new one with the exact steps to reproduce.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.