While with foreach not giving correct value

[Adamhumbug]

Hi All,

Sorry for the rubbish title, wasnt sure how else to word it.

I have a select statement grabbing some info from my DB.

    $sql = "
            SELECT client.id as clientid, company_name, country, DATE_FORMAT(client.date_created, '%d %M %Y') as date_created, sum(total_value) as tval
            from client
            left join quote on client.id = quote.client_id and 1 = quote.open
            group by client.id
            ";
    $stmt = $pdo -> prepare($sql);
    $stmt -> execute();
    $stmt -> bindColumn('company_name', $coName);
    $stmt -> bindColumn('country', $country);
    $stmt -> bindColumn('date_created', $dCreated);
    $stmt -> bindColumn('tval', $tval);
    $stmt -> bindColumn('clientid', $cId);
    $out = "";

i then have a while loop that is comparing the output from another function to match IDs and give a value to the output if they match.

    while ($stmt -> fetch()){
        foreach ($sumOpenAndAcceptedValueByClient as $row){
            if($row['client_id'] == $cId){
                $valByClient = $row['sum(total_value)'];
                echo $cId;
                echo " ".$row['client_id'];
                echo " ".$row['sum(total_value)'];
                echo "<br/><br/>";
            }
        }

It seems to be working, however i have a client in the list that has no value so its sum(total_value) should be blank, actually it wont even show in the array i am foreaching.

The issue is that in the table it is producing, it is giving me the same value of whatever comes before it - you see the £211 in the table image below.

 

I have tried debugging this by echoing the values that are getting passed around which gives me the following output.

1 1 1000

2 2 211

I am therefore not sure why i am getting a value put into the last row.

I have tried adding and else to the for each making it

    while ($stmt -> fetch()){
        foreach ($sumOpenAndAcceptedValueByClient as $row){
            if($row['client_id'] == $cId){
                $valByClient = $row['sum(total_value)'];
                echo $cId;
                echo " ".$row['client_id'];
                echo " ".$row['sum(total_value)'];
                echo "<br/><br/>";
            }else{
                $valByClient = 0;
            }
        }

that gives me the same echo values but now the first row =0 as well as the last.

I am a bit stumped on this and would appreciate your help.

If you need more info or for me to show more code, i will be happy to do so.

As always, thanks in advance.

[Barand]

Does $sumOpenAndAcceptedValueByClient contain the results from another query?

[Adamhumbug]

27 minutes ago, Barand said:

Does $sumOpenAndAcceptedValueByClient contain the results from another query?

it comes from this function

 

function sumOpenAndAcceptedValueByClient(){
    include 'includes/dbconn.php';
    $sql = "SELECT client_id, sum(total_value) from quote where accepted = 1 or open = 1 group by client_id";
    $stmt = $pdo-> prepare($sql);
    $stmt -> execute();
    $out = $stmt -> fetchAll();
    return $out;
}

 

[Adamhumbug]

29 minutes ago, Barand said:

Does $sumOpenAndAcceptedValueByClient contain the results from another query?

Which outputs this

 

array(2) {
  [0]=>
  array(4) {
    ["client_id"]=>
    int(1)
    [0]=>
    int(1)
    ["sum(total_value)"]=>
    string(4) "1000"
    [1]=>
    string(4) "1000"
  }
  [1]=>
  array(4) {
    ["client_id"]=>
    int(2)
    [0]=>
    int(2)
    ["sum(total_value)"]=>
    string(3) "211"
    [1]=>
    string(3) "211"
  }
}

 

[Barand]

It looks like your default fetch mode is

PDO::FETCH_BOTH (default): returns an array indexed by both column name and 0-indexed column number as returned in your result set

which gives you every value twice.

Change your fetch default to PDO::FETCH_ASSOC in your connection options.

[Adamhumbug]

12 minutes ago, Barand said:

It looks like your default fetch mode is

PDO::FETCH_BOTH (default): returns an array indexed by both column name and 0-indexed column number as returned in your result set

which gives you every value twice.

Change your fetch default to PDO::FETCH_ASSOC in your connection options.

i have done that, i am now seeing:

 

array(2) {
  [0]=>
  array(2) {
    ["client_id"]=>
    int(1)
    ["sum(total_value)"]=>
    string(4) "1000"
  }
  [1]=>
  array(2) {
    ["client_id"]=>
    int(2)
    ["sum(total_value)"]=>
    string(3) "211"
  }
}

But still have the extra value.

Edited August 16, 2023 by Adamhumbug

[Barand]

I am a little confused.

1 ) You echo three columns

echo $cId;
echo " ".$row['client_id'];
echo " ".$row['sum(total_value)'];

but your output shows only two...

 

2) both queries are using quotes table, so why not combine them into one and save yourself the foreach() loop?

[Phi11W]

1 hour ago, Adamhumbug said:

SELECT client.id as clientid, company_name, country, DATE_FORMAT(client.date_created, '%d %M %Y') as date_created, sum(total_value) as tval
            from client
            left join quote on client.id = quote.client_id and 1 = quote.open
            group by client.id

BIG problem with this SQL. 

You want the total of total_value for each client, but which values of company_name, country, etc. should the query return? 

On any other DBMS (and even a properly-configured MySQL), this query would be thrown out with an error because the database cannot work it out which one you would want and so gives up.  Your [poorly-configured] MySQL is simply returning any old, arbitrary value for each field that happens to be lying around in its buffer as it reads the data.

 

 

1 hour ago, Adamhumbug said:

$valByClient = $row['sum(total_value)'];

In your query, you've aliased that value as tval, so you would access it using: 

$valByClient = $row['tval'];

 

 

1 hour ago, Adamhumbug said:

foreach ($sumOpenAndAcceptedValueByClient as $row){
            if($row['client_id'] == $cId){

If you have a known "list" of client_id values before you execute the query, consider using the "in" clause to get the database to do the filtering (far more efficiently than your "looping" code ever will).  

SELECT c.id as clientid ... 
from client as c
left join quote as q 
     on c.id = q.client_id 
     and 1 = q.open
WHERE c.id in ( 1, 2, 3, 4 ) 
group by c.id
order by c.id 

 

Regards, 
   Phill  W.

 

[mac_gyver]

1 hour ago, Adamhumbug said:

accepted = 1 or open = 1

what's the difference between accepted and open and why doesn't the first query use accepted = 1 too?

54 minutes ago, Barand said:

but your output shows only two...

i'm betting that the $out = ""; variable in the first posted code is being used to build the output in the image, listing the results by the left-hand ranges (guessing per client), and the echo statements are just there for debugging?

this is going to be a case of - don't show us your non-working attempt and expect us to figure out what's wrong, instead show us sample data and what result you expect from that data.

also -

2 hours ago, Adamhumbug said:

$stmt -> bindColumn('company_name', $coName);

don't waste your time with bindColumn() statements, this is wasted typing. just fetch the data and reference the data by its associative index (column) name.

lastly, why are you preparing a non-prepared query that doesn't have any values being supplied to it? just directly use the ->query() method.

 

[Adamhumbug]

On 8/16/2023 at 3:52 PM, Phi11W said:

BIG problem with this SQL. 

You want the total of total_value for each client, but which values of company_name, country, etc. should the query return? 

On any other DBMS (and even a properly-configured MySQL), this query would be thrown out with an error because the database cannot work it out which one you would want and so gives up.  Your [poorly-configured] MySQL is simply returning any old, arbitrary value for each field that happens to be lying around in its buffer as it reads the data.

I will take your word for this as i am here for help - however, when i have logged out what this sql produces, it seems that i am getting the data that i expect and only this.  When i change the data in the database and re run the query i am stil getting the data that i expect.

 

if it helps here is the complete function.

 

function showClientList(){
    include 'includes/dbconn.php';

    $grandTotal = sumAllQuoteValues();
    $sumOpenAndAcceptedValueByClient = sumOpenAndAcceptedValueByClient();

    echo "<pre>";
    var_dump($sumOpenAndAcceptedValueByClient);
    echo "</pre>";


    $sql = "
            SELECT client.id as clientid, company_name, country, DATE_FORMAT(client.date_created, '%d %M %Y') as date_created, sum(total_value) as tval
            from client
            left join quote on client.id = quote.client_id and 1 = quote.open
            group by client.id
            ";
    $stmt = $pdo -> prepare($sql);
    $stmt -> execute();
    $stmt -> bindColumn('company_name', $coName);
    $stmt -> bindColumn('country', $country);
    $stmt -> bindColumn('date_created', $dCreated);
    $stmt -> bindColumn('tval', $tval);
    $stmt -> bindColumn('clientid', $cId);
    $out = "";




    while ($stmt -> fetch()){
        foreach ($sumOpenAndAcceptedValueByClient as $row){
            if($row['client_id'] == $cId){
                $valByClient = $row['sum(total_value)'];
            }
        }


        if($cId != null){        
        $created = new DateTime($dCreated);
        $now = new DateTime(date("Y-m-d H:i:s"));
        if($created->modify('+3 year') < $now){
            $age = "Long Standing Client";
        }else if ($created->modify('+2 year') < $now) {
            $age = "Established Client";
        }else if ($created->modify('+1 year') < $now) {
            $age = "Second Year";
        }else{
            $age = "New Client";
        }

        $percentageOfGrandTotal = ($valByClient/$grandTotal)*100;
        $roundedPercentage = round($percentageOfGrandTotal,2);


        $out .=<<<EOD
            <tr class='align-middle'>
                <td>
                    <div>$coName</div>
                    <div class='small text-medium-emphasis'><span>$age</span> | Created: $dCreated</div>
                </td>
                <td class='text-center'>
                    <svg class='icon icon-xl'>
                        <use xlink:href='vendors/@coreui/icons/svg/flag.svg#cif-$country'></use>
                    </svg>
                </td>
                <td>
                    <div class='clearfix'>
                        <div class='float-start'>
                            <div type="button"
                            tabindex="0"
                            data-coreui-toggle="popover" 
                            data-coreui-placement="top"
                            data-coreui-custom-class="custom-popover"
                            data-coreui-title="Potential New Revenue Value Of Client"
                            data-coreui-trigger="focus"
                            data-coreui-content="Total open quotes come to £$tval." class='fw-semibold'>£$tval</div>
                        </div>
                        
                        <div type="button"
                        tabindex="0"
                        data-coreui-toggle="popover" 
                        data-coreui-placement="top"
                        data-coreui-custom-class="custom-popover"
                        data-coreui-title="Total Value Of Client"
                        data-coreui-trigger="focus"
                        data-coreui-content="Total open and accepted quotes come to £$valByClient." 
                 class='float-end'><small class='text-medium-emphasis'>£$valByClient</small></div>
                    </div>
                    <div class='progress progress-thin'>
                        <div type="button"
                        tabindex="0"
                        data-coreui-toggle="popover" 
                        data-coreui-placement="top"
                        data-coreui-custom-class="custom-popover"
                        data-coreui-title="Total Value Of Client"
                        data-coreui-trigger="focus"
                        data-coreui-content="This client makes up $roundedPercentage% of total revenue." class='progress-bar bg-success' role='progressbar' style='width: $percentageOfGrandTotal%'></div>
                    </div>
                </td>
                <td class='text-center'>
                    <svg class='icon icon-xl'>
                        <use xlink:href='vendors/@coreui/icons/svg/brand.svg#cib-cc-mastercard'></use>
                    </svg>
                </td>
                <td>
                    <div class='small text-medium-emphasis'>Active/Completed</div>
                    <div class='fw-semibold'>2/8</div>
                </td>
                <td>
                    <div class='dropdown'>
                        <button class='btn btn-transparent p-0' type='button' data-coreui-toggle='dropdown' aria-haspopup='true' aria-expanded='false'>
                            <svg class='icon'>
                                <use xlink:href='vendors/@coreui/icons/svg/free.svg#cil-options'></use>
                            </svg>
                        </button>
                        <div class='dropdown-menu dropdown-menu-end' style=''><a class='dropdown-item' href='#'>Info</a><a class='dropdown-item' href='#'>Edit</a><a class='dropdown-item text-danger' href='#'>Delete</a></div>
                    </div>
                </td>
            </tr>
EOD;
        }
        
    }
    return $out;
}

 

[Adamhumbug]

On 8/16/2023 at 4:29 PM, mac_gyver said:

what's the difference between accepted and open and why doesn't the first query use accepted = 1 too?

i'm betting that the $out = ""; variable in the first posted code is being used to build the output in the image, listing the results by the left-hand ranges (guessing per client), and the echo statements are just there for debugging?

this is going to be a case of - don't show us your non-working attempt and expect us to figure out what's wrong, instead show us sample data and what result you expect from that data.

also -

don't waste your time with bindColumn() statements, this is wasted typing. just fetch the data and reference the data by its associative index (column) name.

lastly, why are you preparing a non-prepared query that doesn't have any values being supplied to it? just directly use the ->query() method.

 

The value in question that is not working correctly is the small grey value - the others are set using queries from other functions called at the top of this one.

 

I take your point on the prepared not being required and in terms of the bind column, i am brand new to PDO - actually made the switch due to @Barand suggestions over the years.  I am still finding my feet.

[mac_gyver]

the way to correct and simplify this is to -

On 8/16/2023 at 8:14 AM, Barand said:

2) both queries are using quotes table, so why not combine them into one and save yourself the foreach() loop?

this will eliminate all the sumOpenAndAcceptedValueByClient() usage. the single query will give you the quote total (accepted and open) and open total (open only) values per client_id. you would then just use those values as you are looping to produce the output.

18 hours ago, Adamhumbug said:

include 'includes/dbconn.php';

i hope you are not doing this in every function that needs a database connection? this is creating multiple database connections per instance of your script and a database connection is one of the slowest operations you can perform. your main code should create one database connection, then supply it to any function that needs it as a call-time parameter. you should also use 'require' for things that your code must have for it to work.

if you eliminate all the bindColumn() statements and just fetch each row into $row, you would reference each column value as $row['column_name'], which is probably how the code was originally written. this will also eliminate the multiple different names you are using for the same meaning data. 

18 hours ago, Adamhumbug said:

if($cId != null){

the id/client_id will never be null from this query since it is from the primary table. this conditional test is unnecessary.