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

Bug: select() & selectSum() not consistent / aligned with docs #3019

Closed
Xirt opened this issue May 19, 2020 · 1 comment
Closed

Bug: select() & selectSum() not consistent / aligned with docs #3019

Xirt opened this issue May 19, 2020 · 1 comment

Comments

@Xirt
Copy link

Xirt commented May 19, 2020

Bug description
Although parameters descriptions on usage of select() and selectSum() are similar on https://codeigniter.com/user_guide/database/query_builder.html, the first method accepts Arrays as input in practice while the latter does not. The easy fix could be to disallow Arrays for select(), but based on the use case for these methods I would expect alignment on acceptance of Arrays. This would prevent many occurences of the method selectSum() in case this is required.

CodeIgniter 4 version
4.0.1

Affected module(s)
CodeIgniter\Database\BaseBuilder

Expected behavior, and steps to reproduce if appropriate
The below (simplified) code reproduces the issue.

$db = \Config\Database::connect();
$builder = $db->table('table');
$builder->select(["field_a", "field_b", "field_c"]); // allowed
$builder->selectSum(["field_d", "field_e", "field_f"]); // not allowed
$query = $builder->groupBy("field_group")->get()

Potential fix
I believe it could be fixed in BaseBuilder::selectSum() by checking for Arrays, looping through them and then feeding them into the method maxMinAvgSum($select, $alias, $type). Potentially this should be done also for other methods (min, max, ...) and obviously documentation should be aligned. Alternatively, this loop could be placed in a seperate method as the logic is likely the same for all these related methods (min, max, ...); only the $type fed into maxMinAvgSum($select, $alias, $type) differs.

public function selectSum($select = '', string $alias = '')
{

    if (is_array($select))
    {

        foreach ($select as $current)
        {

            $current = is_string($current) ? [$current] : $current;
            $this->maxMinAvgSum($current[0] ?? '', $current[1] ?? '', 'SUM');

        }

        return $this;

    }

	return $this->maxMinAvgSum($select, $alias, 'SUM');
}

Context

  • OS: Windows 10
  • Web server: Apache
  • PHP version 7.3.12
@Xirt Xirt added the bug Verified issues on the current code behavior or pull requests that will fix them label May 19, 2020
@paulbalandan
Copy link
Member

These two methods are faux amis (false friends). They appear to have similar signatures but their usage are actually different so they can't have the same handling of parameters due to the nuances each method has.

@paulbalandan paulbalandan removed the bug Verified issues on the current code behavior or pull requests that will fix them label Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants