DB/SQL/Mapper Object dehydrating on save() in 3.7.1

[MDBenson]

I upgraded a working project from 3.6.5 to 3.7.1 and found that, under some circumstances, where I could ->cast() data from a DB/SQL/Mapper object successfully in 3.6.5 and recover the record data after the update (to for example get the new record ID when creating a record), in 3.7.1 the exact same code fails becase the save() is dhydrating the DB/SQL/Mapper object and it's field values are all reverting to null.

Sure enough I rolled fatfree-core back to 3.6.5 and it now works 100% fine again.

I can't post the exact code I have an issue with because it's proprietary property, and annoyingly I wrote a simple example and it works flawlessly the way 3.6.5 did but the issue is 100% there, I can see it in a XDebug step-by-step readback.

I know this isn't amazingly helpful but, as the docvumentation for DB explicitly states that objects must be reset() to clear them I assumed it weas not working as intended in all cases. If @bcosca wants to contact me personally I might be able to share the code 1-to-1 so they can see what's up...

[ikkez]

can you please check if that last commit here is the cause of your issue?!
f3-factory/fatfree-core@0c684da
thanks

[MDBenson]

It would seem it hasnt, no. I have narrowed the issue down to Insert operations. If I use save() to Update an object it seems cast() returns the updated data without a problem. If I use it to Insert a record it sometimes is dehydrating.

I unfortunately don't have time today to test further but when I do I'll get back to here.

[pauljherring]

If I use it to Insert a record it sometimes is dehydrating.

This is probably it: 3.6.5->3.7.1

        /**
        *       Insert new record
        *       @return static
        **/
        function insert() {
                $args=[];
                $actr=0;
                $nctr=0;
                $fields='';
                $values='';
                $filter='';
                $pkeys=[];
                $nkeys=[];
                $ckeys=[];
                $inc=NULL;
                foreach ($this->fields as $key=>$field)
                        if ($field['pkey'])
                                $pkeys[$key]=$field['previous'];
                if (isset($this->trigger['beforeinsert']) &&
                        \Base::instance()->call($this->trigger['beforeinsert'],
                                [$this,$pkeys])===FALSE)
                        return $this;
+               if ($this->valid())
+                       // duplicate record
+                       foreach ($this->fields as $key=>&$field) {
+                               $field['changed']=true;
+                               if ($field['pkey'] && !$inc && $field['pdo_type']==\PDO::PARAM_INT
+                                       && !$field['nullable'])
+                                       $inc=$key;
+                               unset($field);
+                       }
                foreach ($this->fields as $key=>&$field) {
                        if ($field['pkey']) {
                                $field['previous']=$field['value'];
                                if (!$inc && $field['pdo_type']==\PDO::PARAM_INT &&
-                                       empty($field['value']) && !$field['nullable'])
+                                       is_null($field['value']) && !$field['nullable'])
                                        $inc=$key;
                                $filter.=($filter?' AND ':'').$this->db->quotekey($key).'=?';
                                $nkeys[$nctr+1]=[$field['value'],$field['pdo_type']];
                                $nctr++;
                        }
                        if ($field['changed'] && $key!=$inc) {
                                $fields.=($actr?',':'').$this->db->quotekey($key);
                                $values.=($actr?',':'').'?';
                                $args[$actr+1]=[$field['value'],$field['pdo_type']];
                                $actr++;
                                $ckeys[]=$key;
                        }
+                       unset($field);
                }

[pauljherring]

Introduced here: 60ed316#diff-ba2603215b5da69d3f4626f75aefd9d2R437

[ikkez]

I'm unable to reproduce this.

$mapper = new \DB\SQL\Mapper($f3->DB,'news');
$mapper->load();
$mapper->insert();
var_dump($mapper->_id);
$mapper->insert();
var_dump($mapper->_id);
var_dump($mapper->cast());

looks fine

[MDBenson]

Yes, I did the same simple test and it does not occur here either, however it persists in the place I originally found it, so it's something in the depths of my code that is causing it to manifest. I will have time to investigate next week, hopefully.

[ikkez]

I've found something.. when you manually set the id (or other pkey autoincrement field used) to zero (0), it'll insert the record with 0, it'll still get a new id, but it'll not be reloaded because it'll query for WHERE id=0.
Caused by the change here: f3-factory/fatfree-core#290
Easy fix is to not set the ID to 0.. I'm wondering what way to go from here 🤔 is it a bug or not?!

[diapolon]

Hi ikkez,
for me calling $mapper->cast() on empty record return id=0

[joseffb-mla]

At this point it's a philosophy question (which isn't bad). To me, philosophically speaking, it sounds like a bug. If I set an ID to 0 I'm explicitly setting it to false (because there is never a 0 id). Not sure why I would do this but, if I do I expect a 0 to tell the system to do the default behavior.

[geniuswebtools]

@joseffb-mla using the beforeinsert or beforesave event handler you could check for id=0 and clear() the value or set() it to null, and see if that resolves your issue.

[xfra35]

Hi there, I encountered the same issue. We're wondering why would anybody set id to 0, but that could actually be a consequence of trying to set an autoincrement field to NULL.

See this snippet:

// mapper $A is hydrated, mapper $B is dry
// they have a similar structure and we need to import A data to B
$B->copyfrom( ['id'=>NULL] + $A->cast() );
$B->save();
echo $f3->format( 'New row: name={0} id={1} dry={2}', $B->name, $B->id, (int)$B->dry() );

Before f3-factory/fatfree-core@e6f98df, this used to work. Output: New row: name=XXXX id=22 dry=0
After f3-factory/fatfree-core@e6f98df, this started to fail. Output: New row: name= id= dry=1

Now why the id field is seen as 0 (at least in MySQL): when we set $B->id=NULL, the Mapper class actually does $B->id=0. This is a consequence of this line and the fact that in MySQL an autoincrement key cannot be nullable (see).

[xfra35]

Now I'm wondering why we need to write $field['pdo_type']==\PDO::PARAM_INT && empty($field['value']) && !$field['nullable'] to detect autoincrement fields.

Wouln't it be possible to patch the SQL->schema() method to add a $field['autoincrement'] property?

This would allow to fix this issue as well as this line.

[ikkez]

Well the autoincrement fields cannot be NULL in the schema, but when they're NOT NULL you can insert records with the primary key being null or 0:

Insert into testInno value(null, 'roger right');
Insert into testInno value(0, 'willy wonder');
Insert into testMyisam value(null, 'roger right');
Insert into testMyisam value(0, 'willy wonder');

So actually it should not make a difference when inserting. The problem is that reloading the fresh state after insert of that records is based on the value used for id. Since f3-factory/fatfree-core@e6f98df, which was meant as quick fix for accedentially using the wrong primary field, now doesn't accept a 0 anymore. The insert was complete but $inc not set and hence the reason for the empty mapper, as it's no longer using the received autoincremented value, but the populated $filter var with id=$field[id][value], which is 0.
An $field['autoincrement'] flag would be nice, but probably not a trivial task,.. guess we would have done this before, if it was easy. But I think an easy fix is to revert f3-factory/fatfree-core@e6f98df and adjust the if condition once more and add && is_null($field['default']) instead. Because an autoincrement field cannot have a default other than NULL, and there's only 1 of it

[xfra35]

You're right about 0 in MySQL. I didn't know that. Actually:

This behaviour is by design, inserting 0, NULL, or DEFAULT into an AUTO_INCREMENT column will all trigger the AUTO_INCREMENT behaviour.

But it appears that each SQL engine has its own trigger keywords:

• in MySQL: 0, NULL or DEFAULT
• in PostgreSQL, only DEFAULT triggers the autoincrement sequence. 0 inserts 0 and NULL is rejected.
• in SQLite, only NULL triggers the autoincrement sequence. 0 inserts 0 and DEFAULT is rejected.
• in MS SQL, no keyword triggers the autoincrement sequence. One needs either not to specify the autoincrement column or implement a TRIGGER.

See :

• MySQL => https://www.db-fiddle.com/f/iW1hUXWrmZRpru9pZ8WDJC/1
• PostgreSQL => https://www.db-fiddle.com/f/iW1hUXWrmZRpru9pZ8WDJC/4
• SQLite => https://www.db-fiddle.com/f/iW1hUXWrmZRpru9pZ8WDJC/6

As for the $field['autoincrement'] flag, it may be easier than it sounds:

• MySQL => https://www.db-fiddle.com/f/iW1hUXWrmZRpru9pZ8WDJC/3 (column extra)
• PostgreSQL => https://www.db-fiddle.com/f/iW1hUXWrmZRpru9pZ8WDJC/8 (column autoincrement)
• SQLite => https://www.db-fiddle.com/f/iW1hUXWrmZRpru9pZ8WDJC/7 (column sql to be processed in PHP)
• MS SQL => http://sqlfiddle.com/#!18/fe127/12 (column autoincrement)

[ikkez]

If you think you've found an issue, please post more details (code) or open a new issue. thank you

[ikkez]

@stehlo1 there's no need to spam the same lines everywhere. Please tell me what's the issue.

[joseffb-mla]

@stehlo1 It's a volunteer based community not a commercial one. You are the only one who has this issue. Please be respectful in your tone to all the community members who are also reading this. If you have an issue, or better yet a solution, please submit it via an issue ticket so it can be addressed.

…

On Wed, Jun 3, 2020 at 5:18 AM stehlo1 ***@***.***> wrote: These are not the "same lines". Different changes introduced different backward incompatible changes. Don't you understand the code you are changing? @ikkez <https://github.com/ikkez> @bcosca <https://github.com/bcosca> This change corrupted the way the data is inserted into PostgreSQL, namely it is now not inserted at all. This change affects all projects which are using F3. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1175 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKXFTEY4CT6OFS3PWCXACC3RUYIOZANCNFSM4KKUKMYQ> .

[pauljherring]

If you have an issue, or better yet a solution, please submit it via an issue ticket so it can be addressed.

They were too busy throwing their toys out of their pram to have a sensible discussion about it.

[ikkez]

@xfra35 I've implemented your suggestion from #1175 (comment) in the latest commit. It wont work in sqlite2 anymore, only 3.1+ and everything beyond mysql,postgre,mssql,sqlite will fallback to the behaviour pre f3-factory/fatfree-core@0c684da ..soo.. it wasn't that easy, but doable with your preparations from DB fiddle.. thanks for that 👍