#130 GetAssoc issue when column two of row one is null

v5.19
closed-fixed
nobody
None
5
2014-07-04
2014-02-23
Tom Dexter
No

It appears there was an issue introduced between 5.17 and 5.18 caused by this change:

--- adodb.inc.php
+++ adodb.inc.php
@@ -3158,7 +3161,7 @@
            $false = false;
            return $false;
        }
-       $numIndex = isset($this->fields[0]);
+       $numIndex = isset($this->fields[0]) && isset($this->fields[1]);
        $results = array();

If the second column of the first row happens to be null you end up getting a one based array rather than the expected zero based array. Here's an example:

$sql = "SELECT 'test', NULL, 'should be element 1'";
$a = $db->GetAssoc($sql);
print_r($a);

Which results in:
Array
(
[test] => Array
(
[1] =>
[2] => should be element 1
)

)

...with elements 1 and 2 rather than 0 and 1.

It appears that this (using array_key_exists rather than isset, as appears to have been corrected in other places) fixes it:

--- adodb.inc.php-orig  2014-02-23 10:54:36.000000000 -0500
+++ adodb.inc.php   2014-02-23 10:56:17.000000000 -0500
@@ -3164,7 +3164,7 @@
            $false = false;
            return $false;
        }
-       $numIndex = isset($this->fields[0]) && isset($this->fields[1]);
+       $numIndex = array_key_exists(0, $this->fields) && array_key_exists(1, $this->fields);
        $results = array();

        if (!$first2cols && ($cols > 2 || $force_array)) {

Thanks!
Tom

Discussion

  • dregad
    dregad
    2014-02-24

    Thanks for the detailed bug report. I'll check your proposed patch later.

     
  • dregad
    dregad
    2014-02-24

    • Group: v1.0 (example) --> v5.19
     
  • Tom Dexter
    Tom Dexter
    2014-02-24

    I just discovered that my proposed fix causes php errors when the record set is empty (because $this->fields is null rather than an array). Here's a corrected fix:

    --- adodb.inc.php-orig  2014-02-24 12:07:52.000000000 -0500
    +++ adodb.inc.php   2014-02-24 12:08:30.000000000 -0500
    @@ -3168,7 +3168,7 @@
                $false = false;
                return $false;
            }
    -       $numIndex = isset($this->fields[0]) && isset($this->fields[1]);
    +       $numIndex = $this->fields && array_key_exists(0, $this->fields) && array_key_exists(1, $this->fields);
            $results = array();
    
            if (!$first2cols && ($cols > 2 || $force_array)) {
    

    I suppose it could use is_array() there, but I'd guess it's a safe bet that $this->fields will only evaluate to true if it is in fact an array.

    Some design decisions in php blow my mind...like the fact that doing isset($this->fields[0]) when the fields property is null is somehow OK when this isn't....go figure.

    Tom

     
  • dregad
    dregad
    2014-02-24

    Just to be on safe side, I'd rather use is_array(). Also I think the test on $this->fields[1] was added as an imperfect fix for the case when fields[0] was null, so we should get rid of it completely now that we're switching to array_key_exists().

    diff --git a/adodb.inc.php b/adodb.inc.php
    index 376c7a3..fbdc917 100644
    --- a/adodb.inc.php
    +++ b/adodb.inc.php
    @@ -3168,7 +3168,7 @@ http://www.stanford.edu/dept/itss/docs/oracle/10g/server.101/b10759/statements_1
                $false = false;
                return $false;
            }
    -       $numIndex = isset($this->fields[0]) && isset($this->fields[1]);
    +       $numIndex = is_array($this->fields) && array_key_exists(0, $this->fields);
            $results = array();
    
            if (!$first2cols && ($cols > 2 || $force_array)) {
    
     
  • dregad
    dregad
    2014-03-04

    • status: open --> closed-fixed