model->refresh() should return all defined columns

[yueim]

Expected and Actual Behavior

model->refresh() should return all defined columns(fields).

Create a new model, set not null fields, and some fields not assignment any value(using db default value), then save it. and change value, then save it, will throw an exception The record doesn't have a valid data snapshot. Then model->refresh(), and save, PDO exception raised, some fields has default db value, but still notice to set null value: Column 'flag' cannot be null.

SQL schema

DROP TABLE IF EXISTS `demos`;
CREATE TABLE `demos`
(
  `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT COMMENT 'ID',
  `flag` int(10) NOT NULL DEFAULT '1' COMMENT 'Flag',
  `name` varchar(190) COLLATE utf8mb4_unicode_520_ci NOT NULL COMMENT 'Name',
 PRIMARY KEY (`id`) USING BTREE
) ENGINE = InnoDB
  DEFAULT CHARSET = utf8mb4
  COLLATE = utf8mb4_unicode_520_ci COMMENT ='Demos';

Provide minimal script to reproduce the issue

class Demos extends \Phalcon\Mvc\Model {
}

$demo = new Demos();
$demo->name = 'test name';
$result = $demo->save(); // $result === true, everything goes well, record saved into db.

$demo->name = 'new test name';
$result = $demo->save(); // Exception: The record doesn't have a valid data snapshot

$demo->refresh();
$demo->name = 'new test name';
$result = $demo->save(); // Another Exception: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'flag' cannot be null

var_dump($demo->toArray());
// There still only has id and name field after a refresh(). flag field still missing(property does not existed).

MUST DOING THIS WAY TO AVOID EXCEPTION:

$demo = new Demos();
$demo->name = 'test name';
$result = $demo->save(); // $result === true, everything goes well

if ($result) {
  $demo = Demos::findFirst($demo->id);

  $demo->name = 'new test name';
  $result = $demo->save(); // $result === true
}

If some code defined in afterSave() in model, when created then save it:

1. The record doesn't have a valid data snapshot, must be refresh() it.
2. Even after refresh() it, when save it, will using null value to update not existed property, and avoided all default value.

SO, IS THERE A WAY TO REAL REFRESH MODEL WAY?

Details

• Phalcon version: 3.4.2
• PHP Version: 7.2.14
• Operating System: macOS 10.14.2
• Installation type: Compiling from source
• Zephir version (if any):
• Server: Apache
• Other related info (Database, table schema):

[yueim]

@niden thanks for you tagging this issue. hope this can be fixed soon.

[zsilbi]

I ran the same test using the the 4.0.x branch and the exceptions are gone, refresh() works as intented.

But the validation during the second save() fails when the orm.not_null_validations setting is on, because the default values from the database are not present in the Model attributes after the insert.

We could copy the unset default values from the MetaData to the Model after a successful insert, like I did it here: zsilbi@a0c3184, or should we make the validation aware of these defaults?
Currently the validation in _preSave() only allows this when the record doesn't exist.

[yueim]

@zsilbi Thanks for your great work on this framework.
I think let db default value as the field value should be better after refresh().
In most circumstances, db field will define a default value for better performance(at least with MySQL).
So refresh() to retrieve all db default values for all fields is what we expected.

[zsilbi]

In most circumstances, db field will define a default value for better performance(at least with MySQL).
So refresh() to retrieve all db default values for all fields is what we expected.

@cnyyk the default values are present in the MetaData cache even before the insert happens, so we wouldn't really need to refresh()

[niden]

@zsilbi I think the best approach on this would be to copy the unset default values from the MetaData to the Model as you mentioned above. This will solve this issue.

Do you want to do this or should I?

[zsilbi]

Do you want to do this or should I?

@niden I am on it, thanks 👍

[niden]

Resolved in #14088

[yueim]

Thanks again for your great work.
As you mentioned above, the best way is copy all default value from metadata, for less db read.