![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
#30257 closed defect (bug) (wontfix)
wpdb::get_var() returns NULL for existing values that are empty strings
| Reported by: | tyxla | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | Database | Version: | 0.71 |
| Severity: | normal | Keywords: | has-patch |
| Cc: | Focuses: |
Description
I have noticed that using $wpdb->get_var() returns NULL in both of these cases:
- When there is no such value in the database
- When there is a value, but it is an empty string
This appears to be intended and is explicitly defined on the following line: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/wp-db.php#L1870
In my opinion it does not make sense for an empty string to be represented as NULL, as there is a distinguishable difference between NULL and empty string.
Attachments (1)
Change History (10)
#2
@
12 years ago
- Keywords has-patch added
This behavior goes all the way back to when EZ SQL was added in [112]. That means that there is 11 years of code built on top of this, and expecting this behavior. There are even places in core that use get_row() instead to get around this "feature". It might too late to turn back now.
#4
@
12 years ago
Modifying the get_var() will not affect the get_row() calls that @jdgrimes has indicated - these calls will keep working the same way as they have nothing to do with get_var().
I've gone through the entire core and searched for any occurrences of get_var(), and I did not find any single call that depends on the fact that empty strings are considered NULL.
Taking as granted the fact that in PHP NULL == '' is true, the only way to take advantage of the current way get_var() is working is to test its result by using the value and type comparison operator (!== or ===).
I was able to find only one occurrence of such check in the core: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/link-template.php#L1574 but will continue working the same way, as it simply modifies NULL with '', so the result is '', it will keep working the same way like it does now.
Finally, considering all of the above notes I can conclude that I did not find any plausible downsides of having this modification implemented into the core.
#5
@
12 years ago
Thanks for taking the time to do some more research, @tyxla. I realize that this won't affect the get_row() calls, I just wanted to document that because it is related. I wanted to see if there was any pertinent discussion regarding get_var() in the corresponding ticket, but as I said above there wasn't.
I think this would be a good change—I just wanted to see where the code came form and whether there was any reason for it.
It's possible that there are plugins that are relying on this behavior, but even so, I'm not sure that the change could really break anything badly. I guess that's something we should still consider though.
#6
@
12 years ago
- Milestone Awaiting Review
- Resolution → wontfix
- Status new → closed
- Version trunk → 0.71
Thanks for the bug report, tyxla!
I agree that this behaviour isn't ideal, and it's clearly inconsistent with the behaviour of the other get_* methods (except for get_col(), which calls get_var()).
However, due to the age of this behaviour, and the potential to introduce subtle bugs into plugins that expect it, this isn't really a thing we can fix.
If you run into any other behaviour you think should be fixed, please don't hesitate to open a new ticket - we're always happy to discuss these things, even if they're not something we can change. :-)
#7
@
12 years ago
This odd quirk in get_var() is also why we use get_row() for a single field in many other places, such as get_option(), get_site_option(), get_metadata(), etc.
I bet we could change it and few would notice... but @pento is also right that it would break a lot.
#8
@
12 years ago
I understand, although this was not what I expected to be the conclusion here.
Recently I performed a thorough research on the core and did not find a single location where this would break. I guess it might probably break something on a plugin or theme that expects this behavior.
Anyway, I believe that this one wouldn't be the first or the last time where updates would be needed on themes and plugins, in order to make them compatible with the WordPress core. But if the main goal is keeping backward compatibility (and not squashing bugs), you guys are definitely right.
Guess we have to stick with using get_col() and get_row(). Anyway, fixing that issue was worth a shot, at least I rest calm that I tried. :)
The modification that I suggest for fixing this issue.