Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$db->loadObjectList('id', MenuItem::class) fails on Unknown fetch type '8' #254

Open
mahagr opened this issue Sep 1, 2021 · 4 comments
Open

Comments

@mahagr
Copy link

mahagr commented Sep 1, 2021

Steps to reproduce the issue

This example needs Joomla, see issue joomla/joomla-cms#35452

use Joomla\CMS\Menu\MenuItem;

$db = \JFactory::getDbo();
$query = $db->getQuery(true)
    ->select('m.id, m.menutype, m.title, m.alias, m.note, m.path AS route, m.link, m.type, m.level, m.language')
    ->select($db->quoteName('m.browserNav') . ', m.access, m.params, m.home, m.img, m.template_style_id, m.component_id, m.parent_id')
    ->select('e.element as component')
    ->from('#__menu AS m')
    ->join('LEFT', '#__extensions AS e ON m.component_id = e.extension_id')
    ->where('m.menutype = ' . $db->quote($menutype))
    ->where('m.parent_id > 0')
    ->where('m.client_id = 0')
    ->where('m.published >= 0')
    ->order('m.lft');

// Set the query
$db->setQuery($query);

$list = $db->loadObjectList('id', MenuItem::class);

Expected result

The query should work just as it worked in Joomla 3.9/3.10.

Actual result

Exception: Unknown fetch type '8'. This seems to happen when using mysqli but not when using dbo.

Type 8 is same as FetchMode::CUSTOM_OBJECT .

System information (as much as possible)

PHP 8.0.10 / 7.4.23

Additional comments

Following workaround works (replace the last line):

$list = [];
foreach ($db->loadAssocList('id') as $id => $data) {
    $list[$id] = new MenuItem($data);
}
@mahagr
Copy link
Author

mahagr commented Sep 1, 2021

The error happens here:

https://github.com/joomla-framework/database/blob/2.0.0/src/Mysqli/MysqliStatement.php#L542-L546

And this is how it is handled in a couple of other drivers:

https://github.com/joomla-framework/database/blob/2.0.0/src/Pdo/PdoStatement.php#L177
https://github.com/joomla-framework/database/blob/2.0.0/src/Sqlsrv/SqlsrvStatement.php#L464-L467

So it looks like FetchMode::CUSTOM_OBJECT is being handled everywhere else but in mysqli driver. Please check the other drivers, though, as I didn't take a look in every one of them.

@mbabker
Copy link
Contributor

mbabker commented Sep 1, 2021

This is by design. ext/mysqli has inconsistent behavior when working with objects depending on the extension's build and cannot be relied on to handle objects that are not a stdClass, therefore it was an explicit design decision by me to hard block custom object support in the MySQLi driver. From what I remember (since I have long trashed any non-MySQL local database servers, notes, or local clones of any Joomla source repositories since the nearly two-year investment I placed in rewriting the database API), PDO and ext/sqlsrv do not have these same types of issues, hence custom object handling not leading to an exception. #199 and items linked from there explain it in better detail than I could possibly remember at this point.

You are better off not using the custom object mode, and the project would be best off deprecating this mode completely from the database API in favor of a data mapping layer to handle mapping database results into PHP class objects.

@mahagr
Copy link
Author

mahagr commented Sep 2, 2021

Thank you for your response; it makes sense and I have already stopped using the custom objects in my code.

Though your response raises a few questions:

  1. Why Joomla! 4 uses mysqli driver by default?
  2. Why the error message doesn't state that the feature is not supported in mysqli?
  3. Why the feature hasn't been deprecated in the other db drivers?

Personally, I have used PDO the whole time and the issue was only found recently because of that.

@mbabker
Copy link
Contributor

mbabker commented Sep 2, 2021

Just for reference, this GitHub org is the only Joomla owned org I can comment in (and I’d rather leave it this way; it’s better for most people, myself included, that I can’t post in the more visible Joomla repos).

To my recollection, MySQLi is more readily available in a traditional cPanel based setup than PDO is. Hence, the CMS defaulting to it. I generally don’t pay attention to the defaults as I roll new PHP support out on the WHM/cPanel servers I maintain, but it may very well be that PDO is as readily available these days and that it could be considered as the default for a future version. That should be a data driven decision though IMO.

As for improving the error message, as we always say in our various OSS spaces, “pull requests welcome”. Admittedly if I were writing the code today I would’ve used an exception based on that exact scenario, but 2017 me wasn’t doing a lot of “throw Exception with specialized message” types of things.

As for deprecations, that’s on the current technical leads to decide. I would advocate for deprecation of custom object fetching in full since that mode is the most problematic as it relates to a database abstraction layer and creating a cross-platform portable application, and that the generally accepted practice for handling custom objects is through some sort of data mapping layer (whether it’s a full blown ORM or hidden away in an Active Record style implementation like the CMS’ Table class or the Entities Framework package). But, custom objects were always supported through the API and I didn’t want to make any bigger B/C breaks than necessary while refactoring the database driver internals, so I did leave it in place for the drivers where it would reliably work (all of them minus MySQLi and that’s only because of the differences if ext/mysqli is built with libmysql versus mysqlnd).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants