singlestore-labs / singlestoredb-laravel-driver

The official SingleStore Laravel driver.
https://github.com/singlestore-labs/singlestore-laravel-driver
Apache License 2.0
222 stars 22 forks source link

whereNull and whereNotNull don't implement the default MysqlGrammar correctly resulting in exceptions #91

Open larskoole opened 5 hours ago

larskoole commented 5 hours ago

We encountered a bug where if you would pass a DB::raw() to a whereNull() / whereNotNull() it would throw an exception. This is because the SingleStore driver doesn't parse the $where['column'] like in the MysqlGrammar that it extends.

So in short, the SingleStore driver is missing a check to see if a $where['column'] that is passed is an instance of \Illuminate\Contracts\Database\Query\Expression.

Should be a pretty straightforward (and non-breaking) fix by adding $columnValue = (string) $this->getValue($where['column']); to the SingleStore methods.

MysqlGrammar (where getValue() is implemented)

// Illuminate\Database\Query\Grammars\MysqlGrammar

/**
 * Add a "where null" clause to the query.
 *
 * @param  \Illuminate\Database\Query\Builder  $query
 * @param  array  $where
 * @return string
 */
protected function whereNull(Builder $query, $where)
{
    $columnValue = (string) $this->getValue($where['column']);
    if ($this->isJsonSelector($columnValue)) {
        [$field, $path] = $this->wrapJsonFieldAndPath($columnValue);
        return '(json_extract('.$field.$path.') is null OR json_type(json_extract('.$field.$path.')) = \'NULL\')';
    }
    return parent::whereNull($query, $where);
}

/**
 * Add a "where not null" clause to the query.
 *
 * @param  \Illuminate\Database\Query\Builder  $query
 * @param  array  $where
 * @return string
 */
protected function whereNotNull(Builder $query, $where)
{
    $columnValue = (string) $this->getValue($where['column']);
    if ($this->isJsonSelector($columnValue)) {
        [$field, $path] = $this->wrapJsonFieldAndPath($columnValue);
        return '(json_extract('.$field.$path.') is not null AND json_type(json_extract('.$field.$path.')) != \'NULL\')';
    }
    return parent::whereNotNull($query, $where);
}

Grammar (for the getValue() method)

// Illuminate\Database\Grammar

/**
 * Transforms expressions to their scalar types.
 *
 * @param  \Illuminate\Contracts\Database\Query\Expression|string|int|float  $expression
 * @return string|int|float
 */
public function getValue($expression)
{
    if ($this->isExpression($expression)) {
        return $this->getValue($expression->getValue($this));
    }
    return $expression;
}

Current SingleStore implementation (missing the getValue()

// SingleStore\Laravel\Query\Grammar

protected function whereNull(Builder $query, $where)
{
    if ($this->isJsonSelector($where['column'])) {
        [$field, $path] = $this->wrapJsonFieldAndPath($where['column']);
        return '(JSON_EXTRACT_JSON('.$field.$path.') IS NULL OR JSON_GET_TYPE(JSON_EXTRACT_JSON('.$field.$path.')) = \'NULL\')';
    }
    return parent::whereNull($query, $where);
}

protected function whereNotNull(Builder $query, $where)
{
    if ($this->isJsonSelector($where['column'])) {
        [$field, $path] = $this->wrapJsonFieldAndPath($where['column']);
        return '(JSON_EXTRACT_JSON('.$field.$path.') IS NOT NULL AND JSON_GET_TYPE(JSON_EXTRACT_JSON('.$field.$path.')) != \'NULL\')';
    }
    return parent::whereNotNull($query, $where);
}
larskoole commented 4 hours ago

Small addition: the reason of our exception is the fact that $this->isJsonSelector() can't parse an instance of Expression. That's why MysqlGrammar extracts the column value and uses a separate variable to check for the json selector.