Description

Daniel-I-Am

opened on Feb 29, 2024

Issue body actions

The public field SelectStatement::$expr is defined in a PHP docblock as an Expression[]. When parsing a query with a CASE statement, a CaseExpression object is present in this list, despite CaseExpression not extending Expression. This causes issues when explicitly requiring the Expression type as should be returned by SelectStatement::$expr.

Reproduction case

<?php

declare(strict_types=1);

use PhpMyAdmin\SqlParser\Components\Expression;
use PhpMyAdmin\SqlParser\Parser;
use PhpMyAdmin\SqlParser\Statements\SelectStatement;

include_once 'vendor/autoload.php';

$p = new Parser('SELECT a, CASE WHEN b IS NOT NULL THEN 1 ELSE 0 END as c FROM t');

$stmt = $p->statements[0];
if (! $stmt instanceof SelectStatement) {
    throw new Exception('Could not parse select statement');
}

function acceptExpression(Expression $e): void {}

$expressions = $stmt->expr;
foreach ($expressions as $expression) {
    acceptExpression($expression);
}

Expected behavior

I would expect this script to not error out as the $stmt->expr returns Expression[] according to its PHP docblock. Each element should therefore be compatible with Expression in my acceptExpression function.

Real behavior

$ php test.php
PHP Fatal error:  Uncaught TypeError: acceptExpression(): Argument #1 ($e) must be of type PhpMyAdmin\SqlParser\Components\Expression, PhpMyAdmin\SqlParser\Components\CaseExpression given, called in /path/to/test.php on line 22 and defined in /path/to/test.php:18
Stack trace:
#0 /path/to/test.php(22): acceptExpression()
#1 {main}
  thrown in /path/to/test.php on line 18

Activity

1. williamdes commented on Feb 29, 2024

   

   williamdes

   on Feb 29, 2024

   Member

   More actions

   Hello
   What version of sql-parser are you using?
2. Daniel-I-Am commented on Feb 29, 2024

   

   Daniel-I-Am

   on Feb 29, 2024

   Author

   More actions

   This is on the latest release of sql-parser, 5.9.0, with PHP 8.2.

   Reacted by William Desportes
3. williamdes commented on Feb 29, 2024

   

   williamdes

   on Feb 29, 2024

   Member

   More actions

   @MauricioFauth could you please have a look?
4. kamil-tekiela commented on Feb 29, 2024

   

   kamil-tekiela

   on Feb 29, 2024

   Contributor

   More actions

   This was added #88

   I noticed this issue before but I have no idea how to fix this. CaseExpression cannot inherit from Expression, but all of the code is written around the value being only Expression.
5. niconoe- commented on Sep 20, 2024

   

   niconoe-

   on Sep 20, 2024

   Contributor

   More actions

   #583 Defines now that SelectStatement::$expr can be of type CaseExpression[].
   @Daniel-I-Am , your snippet will still cause an error as this is just a change in the PHPDoc, but at least, the PHPDoc will be valid and you could change your behavior to

   function acceptExpression(CaseExpression|Expression $e): void {}

   As @kamil-tekiela said, CaseExpression cannot inherit from Expression, so I think that's the best we could provide.

   Reacted by William Desportes
6. williamdes

   added

   question

   on Dec 21, 2024

Issue actions