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 does not recognize comments #4383

Closed
shurik005 opened this issue Oct 23, 2020 · 3 comments · Fixed by #4397
Closed

SQL parser does not recognize comments #4383

shurik005 opened this issue Oct 23, 2020 · 3 comments · Fixed by #4397

Comments

@shurik005
Copy link

shurik005 commented Oct 23, 2020

Bug Report

Q A
BC Break no
Version 2.12.1

Summary

Function Doctrine\DBAL\Driver\OCI8\OCI8Statement::convertPositionalToNamedPlaceholders() does not skip comments, only literals (single quote) and identificators (double quote).

Current behaviour

Warning: oci_bind_by_name(): ORA-01036: illegal variable name/number
Fatal error: Uncaught Doctrine\DBAL\Driver\OCI8\OCI8Exception: ORA-01008: not all variables bound

How to reproduce

$sql = <<<HERE
/* 
 * test placeholder ?
 */
select dummy as "dummy?" 
  from dual 
 where '?'='?'
-- and dummy <> ?
   and dummy = ?
HERE;
$rows = $oci_connection->fetchAll($sql, ['X']);`

Expected behaviour

As variant of convertPositionalToNamedPlaceholders:

    public static function convertPositionalToNamedPlaceholders($statement) {
        $paramMap = [];
        $statementNamedPlaceholders = '';
        $fragments = preg_split('/(--.*?$)|(\/\*.*?\*\/)|(\'.*?\')|(".*?")/ms',
                $statement, -1, PREG_SPLIT_DELIM_CAPTURE);
        foreach ($fragments as $fragment) {
            if (
                    substr($fragment, 0, 1) == "'" ||
                    substr($fragment, 0, 1) == '"' ||
                    substr($fragment, 0, 2) == '--' ||
                    substr($fragment, 0, 2) == '/*'
            ) {
                $statementNamedPlaceholders .= $fragment;
                continue;
            }
            $subFragments = preg_split('/(\?)/',
                    $fragment, -1, PREG_SPLIT_DELIM_CAPTURE);
            foreach ($subFragments as $subFragment) {
                if ($subFragment != '?') {
                    $statementNamedPlaceholders .= $subFragment;
                    continue;
                }
                $position = count($paramMap) + 1;
                $param = ':param' . $position;
                $paramMap[$position] = $param;
                $statementNamedPlaceholders .= $param;
            }
        }
        return [$statementNamedPlaceholders, $paramMap];
    }
@greg0ire
Copy link
Member

As variant of convertPositionalToNamedPlaceholders:

If that works for you, please consider submitting a PR with that code.

@morozov
Copy link
Member

morozov commented Oct 31, 2020

I believe we could reuse the re2c grammar used by PDO and solve this problem once and for all.

Using the hoaproject/Compiler syntax, it will look like following:

%token  string1        "((\\[\x01-\xFF])|(?!["\\])[\x01-\xFF])*"
%token  string2        '((\\[\x01-\xFF])|(?!['\\])[\x01-\xFF])*'
%token  multichar      :{2,}
%token  escquestion    \?\?
%token  bindchr        :[a-zA-Z0-9_]+
%token  question       \?
%token  comments       /\*([^*]+|[*]+[^/*])*[*]*\*/|--[^\r\n]*
%token  sql            ((?![:\?"'\-\/])[\x01-\xFF])+

#query:
    fragment()*

fragment:
    <string1> | <string2> | <multichar> | <escquestion> | <bindchr> | <question> | <comments> | <sql>

And produce the following result:

>  #query
>  >  token(comments, /* 
 * test placeholder ?
 */)
>  >  token(sql, 
select dummy as )
>  >  token(string1, "dummy?")
>  >  token(sql,  
  from dual 
 where )
>  >  token(string2, '?')
>  >  token(sql, =)
>  >  token(string2, '?')
>  >  token(sql, 
)
>  >  token(comments, -- and dummy <> ?)
>  >  token(sql, 
   and dummy = )
>  >  token(question, ?)

Given that the Hoa project is being abandoned and is not really necessary to be used here, if there is a re2php implementation, we can compile and reuse the original grammar in the DBAL (both in OCI8Statement::convertPositionalToNamedPlaceholders() and SQLParserUtils).

Otherwise, we can manually put together a regex-based parser that uses the regexes above (see #4397).

@morozov morozov linked a pull request Oct 31, 2020 that will close this issue
10 tasks
@morozov morozov added this to the 3.0.0 milestone Nov 14, 2020
@morozov morozov closed this as completed Nov 14, 2020
@morozov morozov changed the title Oracle ORA-01036 - Illegal variable name/number on Dbal 2.12.0 SQL parser does not recognize comments Nov 14, 2020
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants