Parser mistakes with comments

[sinri]

The parser may not well deal with the comments.
Here is an example, as it treat -- as a part of table alias.

Give sql as

SELECT  * FROM ecshop.ecs_order_info 
-- LIMIT 0, 30 
--

process by PhpMyAdmin\SqlParser\Parser

object(PhpMyAdmin\SqlParser\Statements\SelectStatement)#28 (16) {
  ["expr"]=>
  array(1) {
    [0]=>
    object(PhpMyAdmin\SqlParser\Components\Expression)#13 (7) {
      ["database"]=>
      NULL
      ["table"]=>
      NULL
      ["column"]=>
      NULL
      ["expr"]=>
      string(1) "*"
      ["alias"]=>
      NULL
      ["function"]=>
      NULL
      ["subquery"]=>
      NULL
    }
  }
  ["from"]=>
  array(1) {
    [0]=>
    object(PhpMyAdmin\SqlParser\Components\Expression)#11 (7) {
      ["database"]=>
      string(6) "ecshop"
      ["table"]=>
      string(14) "ecs_order_info"
      ["column"]=>
      NULL
      ["expr"]=>
      string(23) "ecshop.ecs_order_info--"
      ["alias"]=>
      NULL
      ["function"]=>
      NULL
      ["subquery"]=>
      NULL
    }
  }
  ["partition"]=>
  NULL
  ["where"]=>
  NULL
  ["group"]=>
  NULL
  ["having"]=>
  NULL
  ["order"]=>
  NULL
  ["limit"]=>
  object(PhpMyAdmin\SqlParser\Components\Limit)#27 (2) {
    ["offset"]=>
    int(0)
    ["rowCount"]=>
    int(30)
  }
  ["procedure"]=>
  NULL
  ["into"]=>
  NULL
  ["join"]=>
  NULL
  ["union"]=>
  array(0) {
  }
  ["end_options"]=>
  NULL
  ["options"]=>
  object(PhpMyAdmin\SqlParser\Components\OptionsArray)#30 (1) {
    ["options"]=>
    array(0) {
    }
  }
  ["first"]=>
  int(0)
  ["last"]=>
  int(12)
}

and rebuild after ensuring the limit expression is appended (by check the statement->limit property, and add it if lack), would get

SELECT  * FROM ecshop.ecs_order_info-- LIMIT 0, 30 

[sinri]

I found the bug existing still in 5.2.0.

The form /* ... */ could be removed, but the -- ... could not.

[williamdes]

@niconoe- Do you want to have a look ?

[sinri]

Here a sample to test.

<?php
// ISSUE 156: https://github.com/phpmyadmin/sql-parser/issues/156

use PhpMyAdmin\SqlParser\Parser;

require_once __DIR__ . '/../vendor/autoload.php';

$sql="select * -- count(*) 
from x.y 
where length(z)<=32
and s<>'F' 
-- and s='T'
and (k=1 /* not decided yet? */ or k=2)
and d=2 /* f */
/* f */ and r=3
order by update_time desc";
try {
    $parser = new Parser($sql);
    //echo json_encode($parser->statements).PHP_EOL;
    //echo json_encode($parser->statements[0]).PHP_EOL;
    echo json_encode($parser->statements[0]->build()) . PHP_EOL;
    echo json_encode($sql) . PHP_EOL;
} catch (Exception $e) {
    echo $e->getMessage() . PHP_EOL . $e->getTraceAsString() . PHP_EOL;
}

[sinri]

I think there would be two ways to resolve this problem.
1, remove the comments when parsing it;
2, ignore the comments when building it.

[sinri]

Another evidence.

select 
    b,
    f(x),
    a, -- ff
    *, -- rr
    * -- count(*) 
from x.y 
left join t.r -- kk
right join t.r on x.y.a=t.r.b -- ll
where length(z)<=32
and s<>'F' 
-- and s='T'
and (k=1 /* not decided yet? */ or k=2)
and d=2 /* f */
/* f */ and r=3
order by update_time desc

It seems that, only the last expression * -- count(*) would cause this issue.

[niconoe-]

I'll try to have a look soon, but I'm not very comfortable with the parser. I better know the lexer from what I've worked on.

In order to not impact a huge ammount of working things here, I suggest to let the current behavior of comments handling in the parser, but simply just ensure that, if we build with comments, all the bytes of characters must be include in the result of the parser (and so, "\r", "\n", "\t", ...).

[sinri]

I tried to make a hot fix for this issue.

[sinri]

I'll try to have a look soon, but I'm not very comfortable with the parser. I better know the lexer from what I've worked on.

In order to not impact a huge ammount of working things here, I suggest to let the current behavior of comments handling in the parser, but simply just ensure that, if we build with comments, all the bytes of characters must be include in the result of the parser (and so, "\r", "\n", "\t", ...).

The idea is okay as the fix done before (7cad6fa) which is mentioned above.

I checked the codes and found it mistook inside the Expression parse method, but it is used not only by parsing SELECT statement. My idea to make hot fix is to check the ExpressionArray and remove the tail comment in last item if exists...