unset() not working within foreach loop

[nlomb_hydrogeo]

 

Hi Freaks

I have an update page for an online groundwater information system. I am using Postgresql/PostGIS.

This is a screenshot of the update page with 2 select boxes. My problem is I don't want the columns 'well_no' or 'geom' to appear in the first select element, i.e. the column to update, as they should NOT be updated.

 

The code for the first selection drop-down list is as follows:

<p> Columns available - select the column to update</p>

            <form class="my-form" method="post">

                <select name='up_column' id='up_column'>

                    <?php

                    try {

                        $result = $pdo->query("select column_name from information_schema.Columns WHERE table_schema = 'public'

                AND table_name = '{$table}' ORDER BY column_name desc");



                        foreach ($result as $row) {

                            unset($row['well_no']);

                            unset($row['geom']);

                            echo "<option value={$row['column_name']}>{$row['column_name']}</option>";

                           

                        }

                    } catch (PDOException $e) {

                        echo "Error: " . $e->getMessage();

                    }

                    ?>
         </select>

 

No need to comment that I am not using prepared statements, as I am in a test environment and will rewrite the query before going into production. Please focus on why my use of unset is not working, as both the forbidden columns are appearing in the select list.

[mac_gyver]

1 hour ago, nlomb_hydrogeo said:

select column_name

the query is SELECTing the column_name, which is why you are echoing column_name to produce the output -

1 hour ago, nlomb_hydrogeo said:

echo "<option value={$row['column_name']}>{$row['column_name']}</option>";

$row['column_name'] will be 'well_no' or 'geom'.

you would create an array with the two values you want to exclude, then because you cannot directly loop over the result set from a query a second time, fetch all the rows of data into a php variable (see the fetchAll() method), then as you are  looping over this array of data, use a conditional test if( !in_array($row['column_name'],['well_no','geom']) ) { produce the output here... } to exclude those two column_name values.

and as was written in one of your previous threads, there no good reason to catch and handle an exception from a SELECT query in your code. just let php catch and handle it. remove the try/catch logic from this query, simplifying the code. also, where this catch logic is at, you won't see the output from it unless you look in the 'view source' in the browser.

[Barand]

Example:

My customer table contains these columns

Columnss
-------------      
syncroid   
State      
Postcode   
id         
CompanyName
City       
Address 

and I want to exclude id and syncroid from the dropdown list.

<?php

$res = $pdo->query("SELECT column_name 
                    FROM information_schema.Columns 
                    WHERE table_schema = 'db1'
                          AND table_name = 'customer' 
                    ORDER BY column_name DESC");
$result = $res->fetchAll(PDO::FETCH_COLUMN);
// define unwanted column names
$exclude = ['id', 'syncroid'];
// remove them
$columns = array_diff($result, $exclude);

?>

<select name='up_column' id='up_column'>

    <?php
        foreach ($columns as $col)  {
            echo "<option>$col</option>\n";
        }
    ?>

</select>

DROPDOWN...

                               

[gizmola]

Obviously there are multiple different ways to solve this issue.  Generically, I'd probably implement a function that does what either of the prior replies to you suggest.

 

The important thing here is that you understand the multiple logic errors in your original code.

 

• You have to understand how a result set is created based on your query.
  • The associative KEY name is going to be the column name.
• Even if your code worked, you are in a foreach loop, so you would have output a broken option value

 

Just in terms of good software engineering practice, your code also suffers from intermixture of "model/data" code and "view/markup". 

I'm sure most of us have done this code at one time in our careers, but it's just bad code.  If you MUST do something like this, I would highly suggest you use alternative syntax for your view code.

This is what i would do to fix your existing code without any substantial change:

 

<?php
   try {
       $result = $pdo->query("select column_name from information_schema.Columns WHERE table_schema = 'public'
                              AND table_name = '{$table}' ORDER BY column_name desc"
       );
       
       $blacklist = ['well_no', 'geom'];
       $rows = [];
       foreach ($result as $row) {
         if (in_array($row['column_name'], $blacklist)) {
           continue;    
         }
         $rows[] = $row;
       }

   } catch (PDOException $e) {
       echo "Error: " . $e->getMessage();
   }
?>

<p>Columns available - select the column to update</p>
    <form class="my-form" method="post">
       <select name='up_column' id='up_column'>
           <?php foreach($rows as $row): ?>
              <option value="<?= $row['column_name'] ?>"><?= $row['column_name'] ?></option>
           <?php endforeach; ?>
       </select>
      

 

 

So hopefully you can see how this has separated concerns, with the query logic moved out of the markup.  One issue with your try-catch, is that it's bad practice to return this type of database error to the end user.  You should just log the actual database error, and return a generic message to the end user if you must.  It really depends on what else is going on with the UI, and what type of end user experience you want. 

 

 

 

[nlomb_hydrogeo]

Thank you mac-gyver, Barand, and gizmola. I now have 3 solutions to choose from!

[jodunno]

On 8/8/2024 at 9:47 PM, gizmola said:

 

foreach ($result as $row) {
  if (in_array($row['column_name'], $blacklist)) {
    break;    
  }
  $rows[] = $row;
}

I think that Gizmola means continue; and not break; so as to continue looping in search of other blacklisted values.

 

 

 

 

[gizmola]

@jodunno Thank you!  100% correct.  I should have had continue in there.  I fixed the code snippet.

[jodunno]

@gizmola I am just trying to be helpful. No need to thank me, my friend. I was going to reply to this thread with similar code to your code but mac_gyver beat me to it. 

Best wishes.

[gizmola]

On 8/15/2024 at 2:52 AM, jodunno said:

@gizmola I am just trying to be helpful. No need to thank me, my friend. I was going to reply to this thread with similar code to your code but mac_gyver beat me to it. 

Best wishes.

It's all good my friend, we appreciate you being a part of the forum.