It isn't necessary to insert default values in drupal_write_record() as the database already knows what they are.

CommentFileSizeAuthor
#9 dont-insert-default-values.patch3.62 KBjbrown
#8 dont-insert-default-values.patch3.61 KBjbrown
#7 dont-insert-default-values.patch1.63 KBjbrown
#4 dont-insert-default-values2.patch1.28 KBjbrown
#2 dont-insert-default-values.patch1.28 KBjbrown
dont-insert-default-values.patch832 bytesjbrown

Comments

Status: Needs review » Needs work

The last submitted patch, dont-insert-default-values.patch, failed testing.

jbrown’s picture

jbrown commented
StatusFileSize
new dont-insert-default-values.patch1.28 KB

Ahh, empty fields in the object need to be populated with default values for the caller.

jbrown’s picture

jbrown commented

Why is it ignored?

jbrown’s picture

jbrown commented
StatusFileSize
new dont-insert-default-values2.patch1.28 KB

Attempting with different filename.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review

Because the issue is set to needs work :)

Crell’s picture

Crell commented
Status: Needs review » Reviewed & tested by the community

I think this makes, sense, if I understand it all correctly. :-)

jbrown’s picture

jbrown commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new dont-insert-default-values.patch1.63 KB

Updated a comment.

Benefits of this patch:

  • Don't bloat queries unnecessarily - performance increase?
  • There is no longer an inconsistency between serialized defaults in database and those that drupal_write_record() writes - see http://drupal.org/node/585894#comment-2779846
jbrown’s picture

jbrown commented
StatusFileSize
new dont-insert-default-values.patch3.61 KB

Updated patch.

Now $record will not be converted to an object if an exception occurs - #731554: drupal_write_record() silently converts an array to an object in case of an exception

jbrown’s picture

jbrown commented
StatusFileSize
new dont-insert-default-values.patch3.62 KB

with new casting coding standards...

Crell’s picture

Crell commented

I'm OK with this assuming the bot is. If that fixes the other issue as well, score.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Needs review » Reviewed & tested by the community

OK, seems like Crell and the bot are good with it. Who could ask for more?

Looks OK to me. I tested and it resolves #731554: drupal_write_record() silently converts an array to an object in case of an exception and I do like this approach better.

aspilicious’s picture

aspilicious commented

#9: dont-insert-default-values.patch queued for re-testing.

aspilicious’s picture

aspilicious commented

*Retesting old patches*

jbrown’s picture

jbrown commented

#9: dont-insert-default-values.patch queued for re-testing.

jbrown’s picture

jbrown commented

Can this be committed?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

This looks like a nice clean-up. I hope our test coverage here is good...

Committed to HEAD.

jbrown’s picture

jbrown commented

Great! This one is also ready: #797680: drupal_write_record() unnecessarily sets primary keys while updating

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.