Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Zend\Log\Write\Db creates non-existing database table columns #2588

Closed
zfbot opened this issue Sep 28, 2012 · 4 comments
Closed

Zend\Log\Write\Db creates non-existing database table columns #2588

zfbot opened this issue Sep 28, 2012 · 4 comments

Comments

@zfbot
Copy link

zfbot commented Sep 28, 2012

Jira Information

Original Issue:ZF2-562
Issue Type:Bug
Reporter:Martin_P
Created:09/15/12
Assignee:weierophinney
Components:Zend\Log

Description

Zend\Log\Writer\Db::eventIntoColumn creates new non-existing database table columns when $value is an array.

protected function eventIntoColumn(array $event)
{
    if (empty($event)) {
        return array();
    }

    $data = array();
    foreach ($event as $name => $value) {
        if (is_array($value)) {
            foreach ($value as $key => $subvalue) {
                // New database table columns are being created here...
                $data[$name . $this->separator . $key] = $subvalue;
            }
        } else {
            $data[$name] = $value;
        }
    }
    return $data;
}```
This is highly illogical, because now this PHP-script decides what database table columns should exist. When an array with other keys is used all of a sudden the script generates new, other database columns and eventually make the entire script fail again because of a database error due to non-existing columns. The user should define the columns to be used and the script should never add non-existing database tabel columns on its own.

More logical would be to define a way to handle arrays with something like json_encode() or serialize(). The most important thing is that it does not break the logging like it does now, so when $value is an array it *must always* return a string, because that is the only type of variable you can write to a database table column.

@zfbot
Copy link
Author

zfbot commented Sep 28, 2012

This issue was ported from the ZF2 Jira Issue Tracker at
http://framework.zend.com/issues/browse/ZF2-562

Known GitHub users mentioned in the original message or comment:
@weierophinney

@ThaDafinser
Copy link
Contributor

        // Transform the event array into fields
        if (null === $this->columnMap) {
            $dataToInsert = $this->eventIntoColumn($event);
        } else {
            $dataToInsert = $this->mapEventIntoColumn($event, $this->columnMap);
        }

Generally developer will define it over the columnMap, so it's fine.

If you use it, it won't create new columns, it maybe tries to insert data into not existing columns and so the database will throw an error, because the table doesn't have this columns.

Maybe it's better to throw an exception, but it will break BC! If someone just used it this way and defined the columns like they are coming in the array....

@ralphschindler
Copy link
Member

Thanks @ThaDafinser, this doesn't seem to be an issue, it seems more like a configuration issue and syncing config with the structure of the table.

@Martin-P
Copy link
Contributor

Martin-P commented Mar 8, 2013

Sorry, I missed all of it due to the transfer to GitHub. I agree now that it is a configuration issue but a non-documented configuration issue. The manual does not even mention this once.
See: http://framework.zend.com/manual/2.1/en/modules/zend.log.writers.html#writing-to-databases

I think this type of behavior should at least be documented in the manual.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants