sql-parser icon indicating copy to clipboard operation
sql-parser copied to clipboard

Incorrect type in SelectStatement::$expr when query has CASE-clause in selected fields

Open Daniel-I-Am opened this issue 1 year ago • 5 comments

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

Daniel-I-Am avatar Feb 29 '24 10:02 Daniel-I-Am

Hello What version of sql-parser are you using?

williamdes avatar Feb 29 '24 10:02 williamdes

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

Daniel-I-Am avatar Feb 29 '24 11:02 Daniel-I-Am

@MauricioFauth could you please have a look?

williamdes avatar Feb 29 '24 12:02 williamdes

This was added https://github.com/phpmyadmin/sql-parser/pull/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.

kamil-tekiela avatar Feb 29 '24 16:02 kamil-tekiela