Per: http://drupal.org/node/403526

Currently if you try to pass a boolean FALSE into a int database column, the DB driver (for MySQL at least) will cast it to a string, which is then an empty string, which produces a PDO exception when trying to save it as an integer.

In D6, we typically used %d, etc, placeholders which did these casts so we never saw this sort of problem.

We have a couple approaches here:

  1. define new/additional placeholders that agin put the onus on the developer to know (Larry claims this will only happen over his dead body).
  2. Make appropriate casts somewhere in the DB driver, ideally with knowledge of the underlying table structure.
CommentFileSizeAuthor
#6 403840-convert-boolean-database.patch2.3 KBDamien Tournoud
#1 convert-bool-403840-1.patch3.1 KBpwolanin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Active » Needs review
FileSize
convert-bool-403840-1.patch3.1 KB

Untested patch.

Status: Needs review » Needs work

The last submitted patch failed testing.

Crell’s picture

Crell CreditAttribution: Crell commented
Status: Needs work » Needs review

Subscribe.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Subscribe, too.

The alternative to this is that module developers now need to cast any TRUE/FALSE values to integers any time they make a query, which is rather... ugh. We've already had to do this a few places in core (content type creation in default.profile springs to mind).

Crell’s picture

Crell CreditAttribution: Crell commented
Status: Needs review » Needs work
Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Status: Needs work » Needs review
FileSize
403840-convert-boolean-database.patch2.3 KB

I think this is pretty important. Let's try this.

Status: Needs review » Needs work

The last submitted patch failed testing.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs work » Needs review

#6: 403840-convert-boolean-database.patch queued for re-testing.

Crell’s picture

Crell CreditAttribution: Crell commented

Let me get this straight...

We bitch and moan on a regular basis at how MySQL silently cases, truncates, and otherwise messes with data rather than giving a useful error message so that we know to fix our code rather than suffer data loss.

So now that we've forced MySQL into strict mode so that it doesn't do that... we want to re-emulate that in PHP space? Does not compute.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented

@Crell - not quite -the PDO driver as we are using it assumes that everything coming in is a string. At the least, i'd at least like the option to treat values as numeric when I know the underlying column is numeric.

Status: Needs review » Needs work

The last submitted patch, 403840-convert-boolean-database.patch, failed testing.

Josh Waihi’s picture

Josh Waihi CreditAttribution: Josh Waihi commented

This wrecks of badness - I mean, a database shouldn't have to typecast variables, they should be in the format you want them to be. You wouldn't expect to run a windows binary on a linux machine. IMO, proper use of var types is better data integrity. I'm with @Crell on this one.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented

This is more a convenience thing than anything else - PHP is very losse about types, so it might be helpful to tell the DB layer rather than manually casting at the PHP layer. In terms of DX I see this as a regression - in D6 I could specify a %d placeholder and many things would work. In D7 it's more fragile.

Josh Waihi’s picture

Josh Waihi CreditAttribution: Josh Waihi commented

Databases error for a good reason: to tell you, you tried to input the wrong data. By morphing the data to match the schema at the db layer, you risk bugs inserting corrupt data and compromising data integrity. How are we suppose to extend data type support for databases when at the same time we're trying to cast variables into a column?

And as for DX? I think data integrity is more important. Do we have to dumb everything down? I don't see Drupal being enterprise with 'features' like casting at the db layer.

joachim’s picture

joachim CreditAttribution: joachim commented

I'm sure I'm not alone in thinking of database fields that hold 0 or 1 as being TRUE/FALSE, so it feels natural (and more readable) to say that in the code.

But I understand the objections to this too. If we wontfix this, it will definitely need documenting both in the 5-6 upgrade notes and the DBTNG API pages on api.d.org and in the handbook.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Priority: Critical » Normal

we're not even sure we want to do anything here. seems pretty uncritical to me.