J4: MysqlDriver::insertObject vs DatabaseDriver::updateObject

[stAn47]

This issue is also a copy of - joomla/joomla-cms#35520

Steps to reproduce the issue

reproduce:
insert a row where DateTime column is '0' -> in this case insertion does not fail and a new row is inserted
update a row where DateTime column is '0' -> an error is provided from mysql (Incorrect datetime value: '0' for column... )

Expected result

1. either both should fail
2. or none should fail

there seems to be a code inconsitancy in Joomla 4 database drivers as insertObject ignores some of the columns with code:
\libraries\vendor\joomla\database\src\Mysql\MysqlDriver.php

foreach (get_object_vars($object) as $k => $v)
		{
			// Skip columns that don't exist in the table.
			if (!array_key_exists($k, $tableColumns))
			{
				continue;
			}

			// Only process non-null scalars.
			if (\is_array($v) || \is_object($v) || $v === null)
			{
				continue;
			}

			// Ignore any internal fields.
			if ($k[0] === '_')
			{
				continue;
			}

			// Ignore null datetime fields.
			if ($tableColumns[$k] === 'datetime' && empty($v))
			{
				continue;
			}

			// Ignore null integer fields.
			if (stristr($tableColumns[$k], 'int') !== false && $v === '')
			{
				continue;
			}

			// Prepare and sanitize the fields and values for the database query.
			$fields[] = $this->quoteName($k);
			$values[] = $this->quote($v);
		}

while updateObject does not use same logic as above:
\libraries\vendor\joomla\database\src\DatabaseDriver.php

foreach (get_object_vars($object) as $k => $v)
		{
			// Skip columns that don't exist in the table.
			if (!\array_key_exists($k, $tableColumns))
			{
				continue;
			}

			// Only process scalars that are not internal fields.
			if (\is_array($v) || \is_object($v) || $k[0] === '_')
			{
				continue;
			}

			// Set the primary key to the WHERE clause instead of a field to update.
			if (\in_array($k, $key, true))
			{
				$where[] = $this->quoteName($k) . ($v === null ? ' IS NULL' : ' = ' . $this->quote($v));

				continue;
			}

			// Prepare and sanitize the fields and values for the database query.
			if ($v === null)
			{
				// If the value is null and we want to update nulls then set it.
				if ($nulls)
				{
					$val = 'NULL';
				}
				else
				{
					// If the value is null and we do not want to update nulls then ignore this field.
					continue;
				}
			}
			else
			{
				// The field is not null so we prep it for update.
				$val = $this->quote($v);
			}

			// Add the field to be updated.
			$fields[] = $this->quoteName($k) . '=' . $val;
		}

i believe both of them should use same definition on columns which are going to be ignored.

example code:
example code - to be run as domain.com/test.php directly

<?php
define('_JEXEC', 1);

if (!defined('_JDEFINES'))
{
	define('JPATH_BASE', dirname(__FILE__));
	require_once JPATH_BASE . '/includes/defines.php';
}
require_once JPATH_BASE . '/includes/framework.php';

// Boot the DI container
$container = \Joomla\CMS\Factory::getContainer();
$container->alias('session.web', 'session.web.site')
	->alias('session', 'session.web.site')
	->alias('JSession', 'session.web.site')
	->alias(\Joomla\CMS\Session\Session::class, 'session.web.site')
	->alias(\Joomla\Session\Session::class, 'session.web.site')
	->alias(\Joomla\Session\SessionInterface::class, 'session.web.site');

// Instantiate the application.
$app = $container->get(\Joomla\CMS\Application\SiteApplication::class);

// Set the application as global app
\Joomla\CMS\Factory::$application = $app;

$q = 'create table if not exists #__test ( `id` INT NOT NULL AUTO_INCREMENT , `mydate` datetime NOT NULL DEFAULT CURRENT_TIMESTAMP, `data` VARCHAR(100) NOT NULL , PRIMARY KEY (`id`)) ENGINE = InnoDB;'; 
$db = JFactory::getDBO(); 
$db->setQuery($q); 
$db->execute(); 

class JTableTest extends JTable {
	var $id = null;
	var $mydate = '0'; 
	var $data = ''; 
	
}

$table = new JTableTest('#__test', 'id', $db); 

/* INSERT */
$datas = array(); 
$datas['id'] = null; 
$datas['mydate'] = '0'; 
$datas['data'] = 'test'; 

$x = $table->bind($datas); 
$x = $table->store(); 


/* UPDATE */
$q = 'select `id` from #__test where `data` = \'test\''; 
$db->setQuery($q); 
$id = $db->loadResult(); 

$datas = array(); 
$datas['id'] = (int)$id; 
$datas['mydate'] = '0'; 
$datas['data'] = 'test'; 

$x = $table->bind($datas); 
$x = $table->store(); 

var_dump($table->getErrors()); 

result:

1. insert is OK
2. update fails with - "Incorrect datetime value: '0' for column 'mydate' at row 1"

best regards, stan

best regards, stan

[alikon]

shouldn't be a sanification task of one of the Table method?
https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Table/Table.php

[richard67]

It is not a sanification to remove columns which might not have a default value from insert statements because that will for sure cause an SQL error. I don't know why things have been done in this way in the driver, but it is definitely wrong.