Issues

ZF-1870: Support default field values in Zend_Db_Table::createRow()

Description

The createRow method does not set default data based on the table metadata. Looking at the source it seems that it was intended, but the feature was never added:


$defaults = array_combine($this->_cols, array_fill(0, count($this->_cols), null));
$keys = array_flip($this->_cols);
$data = array_intersect_key($data, $keys);
$data = array_merge($defaults, $data);

The $defaults variable is currently just filled with nulls. Simple enough to add:


$defaults = array_combine($this->_cols, array_fill(0, count($this->_cols), null));
foreach ($this->_cols as $col) {
    if (isset($this->_metadata[$col]['DEFAULT'])) {
        $defaults[$col] = $this->_metadata[$col]['DEFAULT'];
    }
}

Comments

Assign to Bill Karwin.

Note that {{isset()}} returns {{false}} if the default is 0 or empty string.

Also note that if the default is {{NULL}}, then setting {{$defaults[$col]}} to string 'NULL' won't work -- it'll try to insert a string instead of the SQL {{NULL}} state.

It would be better in cases where the user does not specify a value, to set the insert value to the SQL expression 'DEFAULT'. This is valid and standard SQL syntax. For example:


INSERT INTO mytable (a, b, c) VALUES (10, 'string', DEFAULT)

This automatically picks up the default value for the respective column. With the Zend_Db interface, you'd have to insert a Zend_Db_Expr('DEFAULT').


INSERT INTO mytable (a, b, c) VALUES (10, 'string', DEFAULT)

Ah ha, I didn't know you could do that, thanks. That would certianly be a better way to do it.

Can we resolve this as not an issue?

I don't thinks so. This is still an issue, Bill just provided an alternative (and better) solution to the one I posted.

I agree with Jack. It's still an issue, any non-specified data should default to DEFAULT. I think it would be the most intuitive way to do it.

Okay, thanks guys; I must have misunderstood the comments. :)

Coming across this issue recently, I resolved it by subclassing Zend_Db_Table and Zend_Db_Table_Row.

From what I could tell, the null-filled-data-array arose because of _Row's only means of determining a valid column was through the array_keys of it's $_data property. Without a null-filled $_data property, _Row objects created from createRow() will throw "not in row" exceptions whenever anything is set.

I tried to figure out a way to get around the issues Bill metioned above with prefilling the $data array with the default values but couldn't come up with one. I instead came up with this solution.

Since _Row is already connecting to it's _Table parent class and getting the primary key, I figured it might as well also grab an array of valid column names too, and then use that to validate get & set calls, rather than just the data array.

my subclassed _Db_Table replaces the createRow() function with: {quote} public function createRow(array $data = array()) { // the only difference from this to the parent are the commented-out-lines /* $defaults = array_combine($this->_cols, array_fill(0, count($this->_cols), null)); */ $keys = array_flip($this->_cols); $data = array_intersect_key($data, $keys); /* $data = array_merge($defaults, $data); */

    $config = array(
        'table'   => $this,
        'data'    => $data,
        'stored'  => false
    );

    Zend_Loader::loadClass($this->_rowClass);
    return new $this->_rowClass($config);
}

{quote}

And my _Db_Table_Row class is: {quote} class Tabkey_Db_Table_Row extends Zend_Db_Table_Row_Abstract {

/**
 * The table column names derived from Zend_Db_Adapter_Abstract::describeTable().
 *
 * @var array
 */
protected $_cols;


/**
 * Constructor.
 *
 * Supported params for $config are:-
 * - table       = class name or object of type Zend_Db_Table_Abstract
 * - data        = values of columns in this row.
 *
 * @param  array $config OPTIONAL Array of user-specified config options.
 * @return void
 * @throws Zend_Db_Table_Row_Exception
 */
public function __construct(array $config) {
    parent::__construct($config);

    // Retrieve column names from table schema
    if ($table = $this->_getTable()) {
        $info = $table->info();
        $this->_cols = array_keys((array)$info['metadata']);
    }

}

/**
 * Retrieve row field value
 *
 * @param  string $columnName The user-specified column name.
 * @return string             The corresponding column value.
 * @throws Zend_Db_Table_Row_Exception if the $columnName is not a column in the row.
 */
public function __get($columnName)
{
    $columnName = $this->_transformColumn($columnName);
    if (!array_key_exists($columnName, $this->_data) AND !in_array($columnName, $this->_cols)) {
        require_once 'Zend/Db/Table/Row/Exception.php';
        throw new Zend_Db_Table_Row_Exception("Specified column \"$columnName\" is not in the row");
    }
    return $this->_data[$columnName];
}

/**
 * Set row field value
 *
 * @param  string $columnName The column key.
 * @param  mixed  $value      The value for the property.
 * @return void
 * @throws Zend_Db_Table_Row_Exception
 */
public function __set($columnName, $value)
{
    $columnName = $this->_transformColumn($columnName);
    if (!array_key_exists($columnName, $this->_data) AND !in_array($columnName, $this->_cols)) {
        require_once 'Zend/Db/Table/Row/Exception.php';
        throw new Zend_Db_Table_Row_Exception("Specified column \"$columnName\" is not in the row");
    }
    $this->_data[$columnName] = $value;
}

/**
 * Test existence of row field
 *
 * @param  string  $columnName   The column key.
 * @return boolean
 */
public function __isset($columnName)
{
    $columnName = $this->_transformColumn($columnName);
    return array_key_exists($columnName, $this->_data);
}

} {quote}

Wow, thats a mess. Let me attach these as a file...

I think this patch should fix the issue and be applied before the next minor release. I can commit this if needed to both the trunk and release-1.0. Let me know. Thanks.

Re-organized the column checks in the if() statement

Justin, for that patch to work, you need to make the following modification:

Zend/Db/Table/Abstract.php line 776:

replace:


if ($this->_sequence === true && !isset($data[$pkIdentity])) {

with:


if ($this->_sequence === true && (!isset($data[$pkIdentity]) || $data[$pkIdentity] instanceof Zend_Db_Expr)) {

I messed up :p

Zend/Db/Table/Abstract.php line 761:

find:



replace with:

If a column is part of the primary key, the default will be set to null, otherwise if the column has metadata and the DEFAULT field exists, the column will have a default value of Zend_Db_Expr('DEFAULT')

I just uploaded a new patch which does what I described in my comment note above. I think that should work. It does assume that most people will set their primary key values to null on a single primary key or if it's a composite key, they would set the other, non-autoincrementating parts at least.

Hi Justin Plock nice patch, but
earlier was only $defaults = array_combine($this->_cols, array_fill(0, count($this->_cols), null)); So all columns have null value I think you shoud add this line
else $defaults[$col] = null;

The reason I didn't include the else is because from what I could tell in the Zend_Db_Adapters, $this->_metadata[$col]['DEFAULT'] is always populated no matter what, so the else would never be executed. We can definitely add it though on the rare edge case when DEFAULT doesn't exist.

The one thing I noticed with this patch though is that it does break cases where people were doing (myself included):


public function insert($data) {
  if (!isset($data['created'])) {
    $data['created'] = time();
  }
  return parent::insert($data);
}

This will no longer work because isset($data['created']) will always return true now. You'd need to do:


public function insert($data) {
  if (!isset($data['created']) || ($data['created'] instanceof Zend_Db_Expr)) {
    $data['created'] = time();
  }
  return parent::insert($data);
}

Not sure how big of a deal this is for people.

I have found solution for your issue with insert I use your patch, but now it is in function insert , you can also set up using defaults i am attaching diff raport sorry for the format but i don't have any good tool to do it

This all sounds a bit too much work for me. Surely the default values are only meant to be set when the data is actually committed? In this case, what I believe we should be doing is passing as little information to the 'insert' method as possible.

E.g. If I create a new row, set a 'name' and 'description' field and don't touch anything else, then only those two fields are passed to the INSERT, thereby allowing the database to manage its own defaults and there's less possibility of messing up something on the ZF side.

So the short answer is - track changes to a new row via the 'clean'/'dirty' tags in Zend_Db_Table_Row and for an insert, only send the fields that have been explicitly set. For updates, naturally the entire row of 'clean' and 'dirty' changes would be sent.

I'd like to resolve this ASAP as it's similar to another issue I'm trying to fix - any comments?

Simon: I agree, that sounds like the obvious and ideal solution, more or less the same as Bill Karwin's idea in his seconds comment, only without the DEFAULT keyword. I've no idea why anything more complicated would be required.

That would fix the issue for me.

Committed to trunk - I'll close this issue for now but please let me know ASAP if, for some reason, it's still not satisfying the requirements of this issue.

The major reason for applying this strategy is to allow default values to be evaluated, rather than having them simply passed as a string. Off the top of my head 'CURRENT_TIMESTAMP' is one such defaults keyword from MySQL. If you still wish to explicitly pass default values to a row before it is inserted, the best bet is to subclass 'Zend_Db_Table_Row' and pass in values via the '_update' method.

This issue should have been fixed for the 1.5 release.

Please categorize/fix as needed.

This doesn't appear to have been fixed in 1.5.0. Please update if this is not correct.

Within createRow is the following block:


$config = array(
            'table'    => $this,
            'data'     => $defaults,
            'readOnly' => false,
            'stored'   => false
        );

I believe the value for 'data' should be $data rather than $defaults here.

Regardless of whether support for default values is added or not, I think this is an issue that should be addressed.

Actually, disregard my last comment. Took me a bit to see why $defaults is used instead of $data. Sorry about that.

I'm not sure if it's a good idea to use the DEFAULT value as one will need to do save() and thus refresh() (2 queries) to obtain the value and work with it while the metadata should already have it. And, i'm doing things were one of my row need to know this value before being saved. As for the problem with default value being set while you override insert to put another one, then i think there is a problem in the design. The question is "default value should be set by the database or by code logic", but i guess trying to have both is not a good idea.

Finally, i don't have use the Zend_Data_Table with something else than MySql. MySql doesn't accept expression as default value only constant but for timestamp column. Other RDBMS accept expressions like now() or getdate() for example. I don't know how and if it's possible to obtain this and wrap wih a Zend_Db_Expr when appropriate. In this situation, i must admit that there is no other way than saving and refresh to get the value, but this is clearly defined when designing the database schema.

Ok, I have done some testing and lots of reading and I think I have come up with a solution to this problem/feature request.

Essentially, we have two sources (and two minds) of where default values should come from (when no value for a specific property/column has been supplied) - The code logic and the database itself, both are valid paradigms of data storage & data maintainability.

At current, this method actually does the correct logic, even though the name of the method is a little amibious. (You are not actually getting a row from the database, you are creating an object that IF SAVED, could become a new row in the database.

Keep in mind, defaulting values IS NOT something that should be default behavior, but it should be configured for. Some applications will want to set default values by code, and some applications will defer to the DB for default values (either implied by the table structure, or set upon first save).

So, here is what I propose we do:

1) a Class level property to control how createRow() handles defaults. Its default setting is $_defaultSource = self::DEFAULTS_NONE. Other options would be self::DEFAULTS_CLASS and self::DEFAULTS_DB. DEFAULTS_CLASS will be allowed to get/define default values from a new class property $_defaultData. DEFAULTS_DB will take "static only" values from the database and set them as defaults.

(Another option I am not yet supporting is the ability to set the defaultSource to DEFAULTS_DB_DYNAMIC. This option would allow (at create time, the class to call all the dynamic functions, like now(), timestamp() and any custom expressions to retrieve a value for the new row. Keep in mind though, these values would not represent the time the actual record was inserted, but the time and data returned when the object was created - this could be good or bad, tell me what you think about that).

2) an option parameter to createRow() - public function createRow(array $data = array(), $defaultSource = null) -- options for this will be the same as above, this will allow for explicitly calling for particular functionality at call time instead of at class definition time.

3) in cases where the row has been created and $_defaultSource = DEFAULTS_DB, the save() routine will refresh automatically after the first save().

If we are to do the above, we maintain backwards-compatibility as well as allow for userland centric application data to have just as much control over default values as applications that are database centric thus allowing for the most flexibility for all developers.

Ok - What do you think? Go.

-ralph

I think Ralph's suggestion is well thought-out and a good compromise to implement the two differing behaviors requested while maintaining BC. I say go for it! :D

+1 for Ralph suggestion

This has been implemented in trunk as of r10259.

Below is a sample usage script that demonstrates all of the capabilities. Please let me know if this suites your needs.



<?php


set_include_path(
    // set your include path here to sample run this.
    '/Users/ralphschindler/Projects/repositories/framework/standard/trunk/library/'
    );
require_once 'Zend/Loader.php';
Zend_Loader::registerAutoload();

// change this if need be to something that has access to the test table
$db = Zend_Db::factory('PDO_MYSQL', array('host'=> 'localhost', 'username'=>'test','password'=>'test','dbname'=>'test'));


$db->query('DROP TABLE IF EXISTS `people_test`');
$db->query('
    CREATE TABLE IF NOT EXISTS `people_test` (
      `id` int(11) NOT NULL auto_increment,
      `first_name` varchar(255) NOT NULL,
      `last_name` varchar(255) NOT NULL,
      `political_party` varchar(255) default \'Independent\',
      `gender` enum(\'M\',\'F\') NOT NULL default \'M\',
      `graduation_year` year(2) NOT NULL default \'95\',
      `created_on` date NOT NULL,
      `updated_on` timestamp NOT NULL default CURRENT_TIMESTAMP,
      PRIMARY KEY  (`id`)
    ) ENGINE=MyISAM;
    ');

Zend_Db_Table_Abstract::setDefaultAdapter($db);


/**
 * NO DEFAULTS TEST
 *
 */


class MyTable_NoDefaults extends Zend_Db_Table_Abstract
{
    protected $_name = 'people_test';
}

$tableNoDefaults = new MyTable_NoDefaults();
$rowNoDefaults = $tableNoDefaults->createRow(array('first_name'=>'Joe', 'last_name'=>'Smith'));

echo '// DEMONSTRATE no defaults: ' . PHP_EOL;
Zend_Debug::dump($rowNoDefaults->toArray());





/**
 * DEFAULTS FROM DB 
 *
 */



class MyTable_DefaultsInDb extends Zend_Db_Table_Abstract
{
    protected $_name = 'people_test';
    protected $_defaultSource = self::DEFAULT_DB;
    /**
     * in these values when defaultSource is set to Db, true will implictly take the default
     * value even if its NULLABLE, and setting a property to false will disclude any properties
     * default value explicitly.  This is useful when a property might have a default value for example
     * such as CURRENT_TIMESTAMP
     */ 
    protected $_defaultValues = array('updated_on' => false);
}

$tableDefaultsDb = new MyTable_DefaultsInDb();
$rowDefaultsDb = $tableDefaultsDb->createRow(array('first_name'=>'Joe', 'last_name'=>'Smith'));

echo '// DEMONSTRATE Db Defaults: ' . PHP_EOL;
Zend_Debug::dump($rowDefaultsDb->toArray());





/**
 * DEFAULTS FROM CLASS
 *
 */


class MyTable_DefaultsInClass extends Zend_Db_Table_Abstract
{
    protected $_name = 'people_test';
    protected $_defaultSource = self::DEFAULT_CLASS;
    protected $_defaultValues = array(
        'gender' => 'F',
        'graduation_year' => 97
        );
}

$tableDefaultsClass = new MyTable_DefaultsInClass();
$rowDefaultsClass = $tableDefaultsClass->createRow(array('first_name'=>'Joe', 'last_name'=>'Smith'));

echo '// DEMONSTRATE Class Defaults: ' . PHP_EOL;
Zend_Debug::dump($rowDefaultsClass->toArray());


// cleanup
$db->query('DROP TABLE IF EXISTS `people_test`');

I understand the objective of this feature (which has evolved from the initial report): a new, unsaved Row object has fields missing, or uses the value {{Zend_Db_Expr('DEFAULT')}}, which doesn't have enough information to be useful. Ideally, a developer wants all fields of the Row object to be populated with values, so he can read the fields and choose to change them before a newly created Row object has been saved. Fields he has not specified in the initial {{createRow()}} call should be populated automatically with defaults, either from the table class or from the database metadata.

There are some tricky cases here. How should a field represent auto-increment columns, or columns for which the default is defined using special values like {{USER}} or {{CURRENT_TIMESTAMP}}?

Also triggers can override some fields during insert or update, but there's no way predict what a trigger will do, so don't try to account for this case in the Framework (same decision was made with respect to fetching a primary key value that is altered by a trigger -- i.e. don't handle this case).

I'm not in favor of specifying defaults in the class. That's what the {{DEFAULT}} declaration in the database is for. Allowing defaults to be specified in the class opens the possibility that these defaults will be different from those defined in the database, which leads to confusion.

For example, suppose I {{createRow()}} and leave a couple of columns unspecified, expecting the db's definition for column defaults to take effect, but they aren't -- the class had different defaults of its own. This sort of blurs the definition of "defaults". It's even more confusing because I can use a special value like {{Zend_Db_Expr('DEFAULT')}} as I create the Row object, bypassing the class' idea of defaults, instead using the db's idea of defaults.

If it were me, I wouldn't allow the table class to have the ability to declare defaults at all. Just let the db metadata determine default values.

One of the goals stated for ZF when I joined the project was to avoid the philosophy that people hated about Perl: TMTOWTDI ("There's More Than One Way To Do It"). Customers told Zend that they didn't want infinite choices, they wanted a framework that would guide them to one solution -- the "right" solution -- that is easiest to use, serves most needs, and represents the best practice. ZF will "jump the shark" if it tries to support every suggested idea as optional behavior.

bq. By the way, I have a similar opinion regarding implementing cascading update and delete in the Table class. What a bad idea! You can't implement cascading operations outside the database engine and preserve atomicity and consistency. This is another case of excessive TMTOWTDI. The database already has metadata to handle cascading operations -- safely, in fact -- so this should be the solution the framework guides developers to use. I believe the same is true for column defaults, and I wouldn't like to see the same mistake made here as was made with cascading operations.

Justin Plock also had a comment up there (14/Jan/08) that is important: auto-filling unspecified fields defeats some types of custom Row logic that the developer had implemented. This change therefore breaks existing ZF applications in silent ways. I would say that for this reason alone, the behavior of auto-filling unspecified fields should be postponed until ZF 2.0, since severe BC breakage is permitted only at major versions.

Anyway back to the code Ralph committed, I think there might be some logic errors in the complex {{if()}} statement under the {{DEFAULT_DB}} case. Perhaps some uses of {{&&}} need to be {{||}}.

But I'm not sure why the logic is needed at all. Isn't it simpler and will achieve the same effect to do the following:


if ($defaultSource == self::DEFAULT_DB) {
  foreach ($this->_metadata as $metadataName => $metadata) {
    $defaults[$metadataName] = $metadata['DEFAULT'];
  }
}

I.e. if I configure my table class with {{DEFAULT_DB}} as the source of defaults, shouldn't it use the database metadata for defaults, and ignore any {{$_defaultValues}} defined? And again, unnecessary if defaults are always taken from metadata rather than any class-defined values.

You need to write a lot of tests to prove that your solution behaves as intended in every case. Basically write out a matrix of every permutation of how a PHP table class can be configured with respect to: - Definition of {{$defaultSource}} and {{$_defaultValues}} in the table class; - Definition of {{NULLABLE}} and {{DEFAULT}} in the database table; - Values, if any, you supply for the given column to the {{createRow()}} function, including {{Zend_Db_Expr('DEFAULT')}} and {{Zend_Db_Expr('NULL')}}.

So there are dozens of permutations you need to account for. You don't necessarily need a separate unit test for every single permutation, because some may be mutually exclusive.

Hi, When I reported this issue I had no idea it would become so complicated :) .

First of all, for the same reasons as Bill states, I think the DEFAULTS_CLASS option is a bad idea. It doesn't make any sense to be able to define defaults in two separate places, and I cant think of any situation where you would want to, or need to.

Secondly, am I correct in assuming that DEFAULTS_NONE pre-fills a new row object with nulls, and sends those nulls as part of the INSERT statement (current behaviour), and DEFAULTS_DB does the same, but excludes any rows that have not been explicitly set, thereby allowing the database to use its own defaults? If so, there's currently no method to pre-fill the row object with the values from the metadata, DEFAULTS_METADATA?

Thirdly, I cant see this as an option you'd use on a per row, or even per table basis, it wouldn't make any sense to use different methods within the same application, it'd just cause inconsistency and confusion. Therefore I think it should be set as a static property of the Zend_Db_Table_Abstract class.

So Bill, let me preface all of this by saying that I am of two minds when it comes to this component. The first is the database purist mind. In this sense, I believe it makes the most sense when the database is developed first, and by the book, with a proper schema, proper naming conventions, proper constraints, proper indices, etc. These types of systems are built by senior application developers who have an EXCELLENT understanding of proper database design, or they are designed by database architects -- in these systems, the database is not a product of the application.

On the other hand, I am of a mind that there are developers who build applications in a very "code centric" approach, and to these developers - a database is a datastore, and typically it is developed in tandem with an application. This is not to say this is an improper paradigm of building an application - I am simply stating that this is as a fact of nature. Not everyone can afford to have a DBA (or DB architect) in house.

That being said lets get into the specifics of the issues raised here.

From my understanding - the original issue is asking simply for a means by which the column defaults (presumably when not NULLABLE), can become part of the $_data array prior to any call to save() (thus the unsaved row can still have the notions of these default before any communication with the database itself regarding this row - its an unattached row.) To developers, this is ideal since it will allow them to create new "rows", interact with them at runtime, and have a single call to save(). Currently, if one wants default values defined within the db, it is necessary to make 2 calls to the database - createRow() -> save() -> interact with attached row -> save().

This bug report was filed because the above situation is not ideal. The REASON why this is not ideal is because developers are utilizing Zend_Db_Table as their model, which is not out of the scope of applications built on ZF. Unfortunately, at current, to extend Zend_Db_Table as an application model means that the classes themselves must be extended instead of configured, this means that minimally, people have to extend Zend_Db_Table_Abstract. If someone wanted to have custom "row"/"model" behavior (like defaults), they now have to extend Zend_Db_Table_Row_Abstract, and configure it to work with their corresponding Zend_Db_Table table. Again, when treating Zend_Db_Table as a model directly, the current situation is not ideal.

Lets talk about the proposed solution.

There are technically two features implemented here. 1) the ability to set default values in newly created (unattached rows) from database metadata, and 2) the ability to set default values in newly created unattached rows via configuration. These two new features are MUTUALLY EXCLUSIVE.

For $_defaultSource = DEFAULT_DB, the $_defaultValues array is only used to explicitly include or exclude columns from utilizing database metadata. Why would one want to do this? In two situations. One would want to explicitly INCLUDE a columns default value when that column is NULLABLE. By setting its $_DefaultValue = array('somecolumn' => true), the default value will be used even if the column supports null values. ON THE OTHER hand, a developer would want to explicitly EXCLUDE a value that might be the result of a trigger, user defined function or whatever... An example of this would be excluding default metadata where the columns default is actually CURRENT_TIMESTAMP.

For $_defaultSource = DEFAULT_CLASS, this is a little more straight forward. This option allows a developer to set a list of default values for the columns to be used as the default values for $_data when a new row is created.

Like I said before, these two usage scenarios are mutually exclusive. The source can either be from DB or from CLASS, but not from both. Also keep in mind, that without doing anything, the default behavior is DEFAULT_NONE where no defaults are set. This is an OPT-IN feature, one would turn it on after deciding that defaults in unattached rows are something they need to support in their application and their applications models.

So, to bring this full circle. The reason the are both DEFAULT_DB and DEFAULT_CLASS is to appease the two differing paradigms of development. I understand where one might say that this is TMTOWTDI, but when applying that to an entire paradigm of development, you run into a few issues. I liken it to saying - there is only one way to develop on the desktop - with an IDE. This statement would exclude developers that are still working in notepad - and who is to say that they are not effective individuals.

More comments inline below:

bq. There are some tricky cases here. How should a field represent auto-increment columns, or columns for which the default is defined using special values like USER or CURRENT_TIMESTAMP?

bq. Also triggers can override some fields during insert or update, but there's no way predict what a trigger will do, so don't try to account for this case in the Framework (same decision was made with respect to fetching a primary key value that is altered by a trigger - i.e. don't handle this case).

The feature as implemented is not a silver bullet approach. It is simply there to give the developer as much useful information about a row object as possible so that they are not locked into the 2 query update scheme. See the explicit exclude above.

bq. I'm not in favor of specifying defaults in the class. That's what the DEFAULT declaration in the database is for. Allowing defaults to be specified in the class opens the possibility that these defaults will be different from those defined in the database, which leads to confusion.

You are making 2 assumptions here. The first is that the Zend_Db_Table is simply a gateway between the model and the database - and is actually not the model itself. The second assumption is that the developer has made the decision to use the database as a RDBMS and not simply as a datastore. Most times, developers will put their eggs in the code basket before they put them in the database basket. Maybe thats wrong, I cannot say for sure, I'm simply saying that its a common scenario.

bq. For example, suppose I createRow() and leave a couple of columns unspecified, expecting the db's definition for column defaults to take effect, but they aren't - the class had different defaults of its own. This sort of blurs the definition of "defaults". It's even more confusing because I can use a special value like Zend_Db_Expr('DEFAULT') as I create the Row object, bypassing the class' idea of defaults, instead using the db's idea of defaults.

DEFAULT_DB and DEFAULT_CLASS are mutually exclusive - the values must come from one or the other.

bq. By the way, I have a similar opinion regarding implementing cascading update and delete in the Table class. What a bad idea! You can't implement cascading operations outside the database engine and preserve atomicity and consistency. This is another case of excessive TMTOWTDI. The database already has metadata to handle cascading operations - safely, in fact - so this should be the solution the framework guides developers to use. I believe the same is true for column defaults, and I wouldn't like to see the same mistake made here as was made with cascading operations.

I can understand this from a DBA's perspective. But the same DBA would also advocate against using MyISAM in those situations. Another unfortunate reality is that MyISAM doesnt support constraints and cascading operations. And, considering MyISAM is STILL such a popular storage engine, it seems only prudent that code handle the cascades where the DB was unable to.

bq. Justin Plock also had a comment up there (14/Jan/08) that is important: auto-filling unspecified fields defeats some types of custom Row logic that the developer had implemented. This change therefore breaks existing ZF applications in silent ways. I would say that for this reason alone, the behavior of auto-filling unspecified fields should be postponed until ZF 2.0, since severe BC breakage is permitted only at major versions.

Well, first, it is an OPT-IN feature. So as for existing applications, no defaults will be set unless the developer explicitly changes DEFAULT_NONE to DEFAULT_SOMETHINGELSE. Second, this is an OPT-IN feature ;) In situations where the developer knows there is custom logic in the database, they would not use this feature in this way, and they should resort to the double save (or override createRow to return an attached row).

bq. Anyway back to the code Ralph committed, I think there might be some logic errors in the complex if() statement under the DEFAULT_DB case. Perhaps some uses of && need to be ||.

I think you are referring to the capability of explicitly including or excluding $metadata['DEFAULT'] based on $_defaultValues?

bq. But I'm not sure why the logic is needed at all. Isn't it simpler and will achieve the same effect to do the following:

bq.

 if ($defaultSource == self::DEFAULT_DB) {
   foreach ($this->_metadata as $metadataName => $metadata) {
     $defaults[$metadataName] = $metadata['DEFAULT'];
   }
 }

This would be fine if we ONLY supported implicit inclusion of metatdata default. As I mentioned before, there might be columns you want to explicitly exclude from your default values.


Phew, thats alot to digest :)

Bill, from my perspective, thats the state of the union.

I am gonna pull [~matthew] into this discussion as he might have some insight from the overall "Architect" position. Again, thanks for your critique and insight - you are much more the DB Wizard/Master than I :)

-Ralph

@Jack Sleight - 23/Jul/08 01:54 AM

bq. Secondly, am I correct in assuming...

In Ralph's code, {{DEFAULTS_NONE}} does nothing, so unset fields are left unset. The Row object may give missing fields the Row's idea of default values. Fields that are still unset when they reach the db are given defaults according to db metadata.

Note that specifying {{NULL}} does not activate a db's {{DEFAULT}} if the column permits {{NULL}}. For example:


CREATE TABLE foo (id SERIAL PRIMARY KEY, col INTEGER DEFAULT 123);
INSERT INTO foo (col) VALUES (NULL);
INSERT INTO foo () VALUES (); -- yes this is legal; try it!

The first insert stores {{NULL}}, the second statement stores 123.

Ralph's code pre-fills the row with values, but allows for yet more override behavior so you can control this column-by-column.

The {{$_defaultValues}} array needs to be a non-static member of the Table class, so you can specify this column-by-column treatment of defaults according to the table. This associative array has keys that need to be the names of columns in the database table it maps to. So it can't be a static member of {{Zend_Db_Table_Abstract}}.

Here's a simple test class for the above table {{foo}}:


class foo extends Zend_Db_Table_Abstract
{
  protected $_defaultSource = self::DEFAULT_DB;
  protected $_defaultValues = array('col' => true);
}

$foo = new foo($db);
$row = $foo->createRow();

var_dump($row->toArray());

This uses the database's metadata to assign the default value 123 to {{$foo->col}} upon creation of the object. If you use {{array('col' => false)}} then it does not assign that field, and leaves it unset.

If you use {{$_defaultSource = self::DEFAULT_CLASS}} then the {{$_defaultValues}} array is interpreted differently. The array is a map of field names to literal values to use as defaults. Fields unspecified in this map are not set with defaults.


class foo extends Zend_Db_Table_Abstract
{
  protected $_defaultSource = self::DEFAULT_CLASS;
  protected $_defaultValues = array('col' => 456);
}

If the array itself is not declared in the class, it sets no default values for any fields. Likewise if you declare {{$_defaultSource = self::DEFAULT_NONE}}, no fields are given defaults. This is the behavior of {{Zend_Db_Table_Abstract}}, so Ralph is right that it's opt-in and won't break custom Row behavior unless a developer deliberately changes that property of his class. We assume he does this knowledgeably.

@Ralph Schindler - 23/Jul/08 09:02 AM

bq. I am of a mind that there are developers who build applications in a very "code centric" approach, and to these developers - a database is a datastore...

Yes, I realize that. I think it's a pity, because without understanding how to employ metadata and ACID properties, developers might as well be using flat files. But I acknowledge that many developers are in fact working in that mode.

I also realize that there are legitimate reasons to implement business logic as code in application space instead of as metadata in the database. Developer tools for application space are more sophisticated, you can make rules behave dynamically at runtime, etc. Not to mention that most people are more comfortable programming in one language at a time.

bq. Currently, if one wants default values defined within the db, it is necessary to make 2 calls to the database - createRow() -> save() -> interact with attached row -> save(). bq. The REASON why this is not ideal is because developers are utilizing Zend_Db_Table as their model...

I agree that using a Table as synonymous with a Model is not appropriate, but I don't agree that this is the reason for the feature request of populating the row with default values. A Row object is still an object whether it's used as the Model or not. Filling the unsaved object's fields with its default values is the most staightforward OO interface for seeing what value is expected to be inserted into your database.

bq. One would want to explicitly INCLUDE a columns default value when that column is NULLABLE. By setting its $_DefaultValue = array('somecolumn' => true), the default value will be used even if the column supports null values. ON THE OTHER hand, a developer would want to explicitly EXCLUDE a value that might be the result of a trigger, user defined function or whatever... An example of this would be excluding default metadata where the columns default is actually CURRENT_TIMESTAMP.

Right. Those are reasonable use cases.

bq. Like I said before, these two usage scenarios are mutually exclusive. The source can either be from DB or from CLASS, but not from both.

Interesting. I wonder if one could unify all three modes, and eliminate the need for the {{$_defaultSource}} property. In other words, the behavior is determined solely by the value in {{$_defaultValues}}:


class foo extends Zend_Db_Table_Abstract
{
  protected $_defaultValues = array(
    'col1' => 456,   -- set field value to specified literal
    'col2' => true,  -- set field value based on table metadata
    'col3' => false, -- do not set field value
                     -- do not set field value for missing 'col4'
  );
}

This allows the developer to use either class-defined or db-defined defaults on a column-by-column basis. This may seem like it runs even more afoul of the offensive TMTOWTDI. But on the other hand, it simplifies the usage by eliminating {{$_defaultSource}}.

bq. Also keep in mind, that without doing anything, the default behavior is DEFAULT_NONE where no defaults are set. This is an OPT-IN feature...

Yes, fair enough; I did not make that connection in my previous comment.

bq. The second assumption is that the developer has made the decision to use the database as a RDBMS and not simply as a datastore.

Sigh. Yes, that is my assumption. Unfortunately you are correct that the prevailing state of web software development eschews RDBMS principles. I think it is to their disadvantage, and it could be the role of frameworks to guide them to better behavior, but at this point it's like pushing water uphill.

I apologize for being negative. I just read a bunch of blogosphere announcements for "Drizzle" -- a fork of MySQL to "refactor" out a lot of the features that make it SQL-compliant. I say fie on them!


Okay, I retract my objection to this feature. If this is what people want, then go for it. I'm satisfied that the implementation is opt-in, so it won't break BC. Please consider the change I suggested to make the usage simpler.

@Bill Karwin - 23/Jul/08 01:26 PM

{quote}In Ralph's code, DEFAULTS_NONE does nothing, so unset fields are left unset. The Row object may give missing fields the Row's idea of default values. Fields that are still unset when they reach the db are given defaults according to db metadata.{quote}

Sorry, I got confused because Zend_Db_Table's behaviour when I created this issue vs. 1.5.2 is different. When I created this the row was pre filled with NULLs and those NULLs were sent in the INSERT statement, thereby stopping the DB using it's own defaults. Thanks to Simon Mundy's change on the 17/Feb/08 this is no longer the case, only changed fields are sent in the INSERT, allowing the table defaults.

{quote}The $_defaultValues array needs to be a non-static member of the Table class, so you can specify this column-by-column treatment of defaults according to the table. This associative array has keys that need to be the names of columns in the database table it maps to. So it can't be a static member of Zend_Db_Table_Abstract.{quote}

To clarify, I didn't mean the $_defaultValues property, I meant the $_defaultSource property, which could be set as a static property of Zend_Db_Table_Abstract as a global setting for all tables.

@Everyone

As far as I'm concerned this issue was resolved by Simon Mundy's change on the 17/Feb/08. I've no idea why THIS issue was re-opened, because what we're now discussing is really a completely separate feature. Regardless of that, the current (1.5.2) behaviour does exactly what I was after when I originally created the issue, and so long as this new feature is OPT-IN (which I shouldn't think I'll be doing), I have nothing further to add.

This was fixed in 1.7.0