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

[4.0] Backend: Duplicated query #27232

Closed
810 opened this issue Dec 10, 2019 · 9 comments
Closed

[4.0] Backend: Duplicated query #27232

810 opened this issue Dec 10, 2019 · 9 comments

Comments

@810
Copy link
Contributor

810 commented Dec 10, 2019

Steps to reproduce the issue

Enable debug.

Login backend.

Expected result

1x the query

Actual result

2x the query

System information (as much as possible)

Nightly build

Additional comments

UPDATE `x3j0d_update_sites`
SET `enabled` = :enabled
WHERE `update_site_id` = :id

UPDATE `x3j0d_update_sites`
SET `enabled` = :enabled
WHERE `update_site_id` = :id
@richard67
Copy link
Member

As you can see the query uses bind variables (the things starting with colon). These may have different values, and for sure they have at least different values for :id, because the statement is used to update status of an update site for a particular if, and so it is not a duplicate query.

@richard67
Copy link
Member

P.S. Please close your issue due to reasons stated in my previous comment. I could close it for you, but I prefer not to use this power.

@810
Copy link
Contributor Author

810 commented Dec 10, 2019

when i set it to old id, then i got just 1 query

$db = $this->parent->getDbo();
		$query = $db->getQuery(true)
			->update($db->quoteName('#__update_sites'))
			->set($db->quoteName('enabled') . ' = :enabled')
			->where($db->quoteName('update_site_id') . ' = ' . $update_site_id)
			->bind(':enabled', $enabled, ParameterType::INTEGER);
		$db->setQuery($query);

@mbabker
Copy link
Contributor

mbabker commented Dec 10, 2019

Again, just because the text of the query is the same does not mean it is a duplicated query. Please confirm whether the query is being executed for a single row multiple times or for multiple rows. Without a confirmation that the query is being executed multiple times for one row, this is not an issue report that can be processed in any way other than closing it.

@810 810 closed this as completed Dec 10, 2019
@richard67
Copy link
Member

That's the disadvantage of parametrized queries or prepared statements or however you call it: They are parsed on server side (including bind variable replacement), and so in debug we don't see the final query. Not sure if this can be changed to be parsed on client side. I've once seen such option in an ODBC driver, I think I remember, but I'm not 100% sure.

@Quy
Copy link
Contributor

Quy commented Dec 10, 2019

@richard67 See #26135

@richard67
Copy link
Member

@Quy I saw that one. But I'm not sure if there is a solution, except of replacing the bind vars on client side just for debugging, but there would not be a guarantee that we see the same query as the server does. It just needs some mistake when doing thatz replacement, and you see something in the debugger which is not equal to what the db server executes.

@mbabker
Copy link
Contributor

mbabker commented Dec 11, 2019

@richard67 there are already plenty of libraries that have logic for replacing parameters with their parameterized values (the Laravel extension of the PHP Debug Bar used in 4.0 has support for this, and somewhere within the code used by the DoctrineBundle for Symfony is code to show an executable query). So it's really not as brittle as you might think it is.

@richard67
Copy link
Member

@mbabker That‘s good to know. That will save a lot of work. Thanks for the info.

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

No branches or pull requests

5 participants