bcit-ci / CodeIgniter

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
18.27k stars 7.64k forks source link

BUG: set_update_batch / update_batch, getting index #6284

Open gidzr opened 2 weeks ago

gidzr commented 2 weeks ago

@narfbg

I think this is a bug.

To recreate:

$this->db->set_update_batch($dataArrBatch,'index_col')->update_batch('table_name');

Gives error:

"You must specify an index to match on for batch updates."

Hypothesis:

The index parameter in the set_update_batch() function is not parsing to the ->update_batch function

Disclaimer:

Maybe I've completely overlooked the correct usage and assumed the wrong behaviours.. so if that's the case I apologise in advance.

My workaround / patch:

OLD VERSION

    public function set_update_batch($key, $index = '', $escape = NULL)
    {
        $key = $this->_object_to_array_batch($key);

        if ( ! is_array($key))
        {
            // @todo error
        }

        is_bool($escape) OR $escape = $this->_protect_identifiers;

        foreach ($key as $k => $v)
        {
            $index_set = FALSE;
            $clean = array();
            foreach ($v as $k2 => $v2)
            {
                if ($k2 === $index)
                {
                    $index_set = TRUE;
                }

                $clean[$k2] = array(
                    'field'  => $this->protect_identifiers($k2, FALSE, $escape),
                    'value'  => ($escape === FALSE ? $v2 : $this->escape($v2))
                );
            }

            if ($index_set === FALSE)
            {
                return $this->display_error('db_batch_missing_index');
            }

            $this->qb_set_ub[] = $clean;
        }

        return $this;
    }

PATCHED VERSION

    public function set_update_batch($key, $index = '', $escape = NULL)
    {
        $key = $this->_object_to_array_batch($key);

        if ( ! is_array($key))
        {
            // @todo error
        }

        is_bool($escape) OR $escape = $this->_protect_identifiers;

        foreach ($key as $k => $v)
        {
            $index_set = FALSE;
            $clean = array();
            foreach ($v as $k2 => $v2)
            {
                if ($k2 === $index)
                {
                    $index_set = TRUE;
                }

                $clean[$k2] = array(
                    'field'  => $this->protect_identifiers($k2, FALSE, $escape),
                    'value'  => ($escape === FALSE ? $v2 : $this->escape($v2)),
                    'index'  => $index_set ? $index : NULL,    /////<-------------------- NEW LINE
                );
            }

            if ($index_set === FALSE)
            {
                return $this->display_error('db_batch_missing_index');
            }

            $this->qb_set_ub[] = $clean;
        }

        return $this;
    }

    // --------------------------------------------------------------------

OLD VERSION

    public function update_batch($table, $set = NULL, $index = NULL, $batch_size = 100)
    {
        // Combine any cached components with the current statements
        $this->_merge_cache();

        if ($index === NULL)
        {
            return ($this->db_debug) ? $this->display_error('db_must_use_index') : FALSE;
        }

        if ($set === NULL)
        {
            if (empty($this->qb_set_ub))
            {
                return ($this->db_debug) ? $this->display_error('db_must_use_set') : FALSE;
            }
        }
        else
        {
            if (empty($set))
            {
                return ($this->db_debug) ? $this->display_error('update_batch() called with no data') : FALSE;
            }

            $this->set_update_batch($set, $index);
        }

        if (strlen($table) === 0)
        {
            if ( ! isset($this->qb_from[0]))
            {
                return ($this->db_debug) ? $this->display_error('db_must_set_table') : FALSE;
            }

            $table = $this->qb_from[0];
        }

        // Batch this baby
        $affected_rows = 0;
        for ($i = 0, $total = count($this->qb_set_ub); $i < $total; $i += $batch_size)
        {
            if ($this->query($this->_update_batch($this->protect_identifiers($table, TRUE, NULL, FALSE), array_slice($this->qb_set_ub, $i, $batch_size), $index)))
            {
                $affected_rows += $this->affected_rows();
            }

            $this->qb_where = array();
        }

        $this->_reset_write();
        return $affected_rows;
    }

PATCHED VERSION

    public function update_batch($table, $set = NULL, $index = NULL, $batch_size = 100)
    {
        // Combine any cached components with the current statements
        $this->_merge_cache();

                    /////<-------------------- REMOVE INDEX SECTION AND PUT BELOW

        if ($set === NULL)
        {
            if (empty($this->qb_set_ub))
            {
                return ($this->db_debug) ? $this->display_error('db_must_use_set') : FALSE;
            }
        }
        else
        {
            if (empty($set))
            {
                return ($this->db_debug) ? $this->display_error('update_batch() called with no data') : FALSE;
            }

            $this->set_update_batch($set, $index);
        }

        if (strlen($table) === 0)
        {
            if ( ! isset($this->qb_from[0]))
            {
                return ($this->db_debug) ? $this->display_error('db_must_set_table') : FALSE;
            }

            $table = $this->qb_from[0];
        }

        // Batch this baby
        $affected_rows = 0;
        for ($i = 0, $total = count($this->qb_set_ub); $i < $total; $i += $batch_size)
        {

            $qb_set_ub = current($this->qb_set_ub[$i]);    /////<-------------------- NEW LINE

            $index = $index !== NULL ? $index : ($qb_set_ub['index'] !== NULL ? $qb_set_ub['index'] : NULL) ;      /////<-------------------- NEW LINE

            if ($index === NULL)    /////<-------------------- SECTION THAT WAS REMOVED FROM ABOVE
            {
                return ($this->db_debug) ? $this->display_error('db_must_use_index') : FALSE;
            }

            if ($this->query($this->_update_batch($this->protect_identifiers($table, TRUE, NULL, FALSE), array_slice($this->qb_set_ub, $i, $batch_size), $index)))
            {
                $affected_rows += $this->affected_rows();
            }

            $this->qb_where = array();
        }

        $this->_reset_write();
        return $affected_rows;
    }

ps. I love CI-3.. query builder is awesome.. I am not moving to CI4 because all I need is simple MVC classes, awesome query builder, simple global post retriever, simple DB integration and I've also customised the query builder for JSON functions. I don't need any other bells and whistles like form validation, front-end stuff, etc. I feel that the power of a php framework is the framework / architecture / security / extensibility, .. it just needs to accommodate and stack other tech and take care of cyber threats.

jamieburchell commented 2 days ago

Think you have the usage incorrect.

set_update_batch($key[, $value = ''[, $escape = NULL]])

Note that the second parameter isn't for an index column to be specified but for a single value in the case where $key is a single field.

update_batch()

The first parameter will contain the table name, the second is an associative array of values, the third parameter is the where key.

So you probably need to specify your index column in the third parameter of update_batch().

Try:

$this->db->set_update_batch($dataArrBatch)->update_batch('table_name', NULL, 'index_col');

Or

$this->db->update_batch('table_name', $dataArrBatch, 'index_col');

https://www.codeigniter.com/userguide3/database/query_builder.html