Devs idea of SQL

Tue, 14 May 2019 09:26:10 +0200

buckaroo@hub.netzgemeinde.eu

Take a look at this gem - it's part of a shopping software

SELECT DISTINCT s0_.id AS id_0, s1_.name AS name_1, s2_.description AS
description_2, s3_.countryname AS countryname_3, s0_.id AS id_4 FROM s_user
s0_ 
LEFT JOIN s_core_shops s1_ ON s0_.subshopID = s1_.id 
LEFT JOIN
s_user_addresses s4_ ON s0_.default_billing_address_id = s4_.id 
LEFT JOIN
s_core_countries s3_ ON s4_.country_id = s3_.id 
LEFT JOIN s_user_attributes
s5_ ON s0_.id = s5_.userID 
LEFT JOIN s_core_customergroups s2_ ON s0_.customergroup = s2_.groupkey ORDER BY
s0_.id DESC LIMIT 20 OFFSET 0;

Five LEFT JOINs to get 20 customers. This query takes on a non-ssd machine about 6 Minutes(!) to run and creates temporary tables about the size of 80 MBs - just to get 20 entries(!). One of the ideas the devs came up: Just throw RAM and SSD at it, Jesus, what happened to optimizing queries?

english

show all 13 comments

Tue, 14 May 2019 09:33:05 +0200

Martin Seeger

masek@pluspora.com

Jesus, what happened to optimizing queries?

Got killed by "Works on my machine" ;-)

Tue, 14 May 2019 09:42:16 +0200

looks like s***ware :-(

Tue, 14 May 2019 09:42:45 +0200

Mark Nowiasz

buckaroo@hub.netzgemeinde.eu

@Tors10 Bingo :thumbsup:

Tue, 14 May 2019 09:45:24 +0200

and i am wondering why this shop works so slowly

Tue, 14 May 2019 12:59:15 +0200

Jochen Spieker

jochen@pluspora.com

I don't think there is anything wrong with this statement. Of course, the dev should have made sure that the schema is prepared for these joins. Given the right indexes, and database should be able to deliver a result in very few milliseconds.

Tue, 14 May 2019 13:19:05 +0200

Mark Nowiasz

buckaroo@hub.netzgemeinde.eu

@Jochen Spieker Oh yes, there is lot of wrong things wrong (not counting DISTINCT and limit):

# Query_time: 92.799554  Lock_time: 0.000203  Rows_sent: 20  Rows_examined: 10007940  Rows_affected: 0
# Bytes_sent: 1359  Tmp_tables: 1  Tmp_disk_tables: 1  Tmp_table_sizes: 96076020
# InnoDB_trx_id: 18B9868C5
# QC_Hit: No  Full_scan: Yes  Full_join: Yes  Tmp_table: Yes  Tmp_table_on_disk: Yes

10 Millions row created by this abomination, more than 90 MB tmp table, and for what? 20 result rows. This is not my definition of "not anything wrong".

Tue, 14 May 2019 13:40:29 +0200

Mark Nowiasz

buckaroo@hub.netzgemeinde.eu

Oh, and as a bonus. Guess what this query is used for? To browse through the list of customers in the backend, hence the "LIMIT 20 OFFSET 0". So if you click on the next page (next 20 customers), you'll get the monster query executed again, only with "LIMIT 20 OFFSET 20".

Tue, 14 May 2019 14:58:11 +0200 zuletzt bearbeitet: Tue, 14 May 2019 14:58:56 +0200

Nemethon

nemethon@hub.netzgemeinde.eu

Es lohnt halt immer wenn man sich vorher viel Gedanken über die Struktur einer Datenbank macht. Das alleine mindert schon die Abfragen und vermeidet etwas wie oben.. ;)

Die meiste Zeit bei meinen Projekten ging für die Planung der Datenbank drauf.

Tue, 14 May 2019 16:10:17 +0200

Christoph S

christophs@pluspora.com

Wir haben damals in der UNI noch (SQL) Abfragen optmieren gelernt üner die Regeln der Relationalen Algebra , so ähnlcih wie das hier dargestelt ist: https://www.geeksforgeeks.org/query-optimization/

Tue, 14 May 2019 22:16:59 +0200

Jochen Spieker

jochen@pluspora.com

So how would you optimize such a statement? AFAICT you could get rid of one JOIN because s_user_attributes is unused. But I still think any database should be able to do this and the rest quickly given the right indexes. I assume that s_user.id is a primary key which should make the ORDER BY and LIMIT easy.

And how would you implement paging over a result set over distinct HTTP requests? You could argue that this functionality does not make sense in the first place if you are talking about more than a few hundred users. But requirements are, unfortunately, often not in a developer's power to decide. Also, if you are customizing off-the-shelf software, you typically do not have (much) control over the schema.

Again with slightly better formatting:
SELECT DISTINCT s0_.id AS id_0, s1_.name AS name_1, s2_.description AS description_2, s3_.countryname AS countryname_3, s0_.id AS id_4
FROM s_user s0_
LEFT JOIN s_core_shops s1_ ON s0_.subshopID = s1_.id
LEFT JOIN s_user_addresses s4_ ON s0_.default_billing_address_id = s4_.id
LEFT JOIN s_core_countries s3_ ON s4_.country_id = s3_.id
LEFT JOIN s_user_attributes s5_ ON s0_.id = s5_.userID
LEFT JOIN s_core_customergroups s2_ ON s0_.customergroup = s2_.groupkey
ORDER BY s0_.id DESC LIMIT 20 OFFSET 0

I think you could argue that you should not be doing what the developer is doing in the first place. But I fail to see how the statement itself could be done any better except for the one superfluous JOIN. Care to enlighten me?

(And yes, I work in software development and know this very situation quite well. I help run an e-commerce site with tens of million registered users and a few tables with 10e8 rows. I have my fair share of "It was fast in my VM" stories, but this does not look like one to me or at least that is not the whole story.)

Wed, 15 May 2019 08:08:35 +0200

Mark Nowiasz

buckaroo@hub.netzgemeinde.eu

Well, first of all it's not my job to optimize such a statement - I'm primarily ops, not dev. Furthermore it's not something one of our devs has created, or a plugin, or whatever, nope, it's part of the software's core. Additionally the list worked perfectly fine, only after updating it became unusable. If this had been OS software I might be inclined in digging into the database and creating a efficient statement, but since it's commercial software this would be a fool's game - and again, not my job. Our job was determining why the fuck the list timed out. Asking me for fixing this monstrosity (I myself would take look first if the JOIN conditions couldn't be optimized) is tantamount to chef who cooked something completely rotten asking the customer how he would prepare the dish.

Since in the previous version the customer's list was working fine there is a better and more efficient way than creating temporary tables containing 10 millions of entries just to retrieve 20 results. Such infefficient queries are usually a sign that either the dev in question didn't think how the query would run on a larger database, i.e. didn't think at all about optimization, just found a short SQL query with 5 LEFT JOINS doing the trick or the dev used an ORM and didn't care at all in optimizing the generated query. Either way he had no idea how this would effect an non-trivial database.

Oh, BTW, the database in question isn't very big - roughly 20 GB (inlcuding indexes and so on). Half the size of my hub's database, and I'm used to shop databases ranging from 50 to 2 TBs. So how do you think this would work on a larger shop?

Anyway, if this piece of shit is the only way of getting the information (which I seriously(!) doubt), there a couple of things you could do: For example, a new table with the results. Either create this table every hour (which would make real time lists impossible, but what the hell) or use triggers (which I really don't like) to update said table when the tables in question change, etc. Granted, not very elegant, still more effiicent than creating 10 Million rows.

Wed, 15 May 2019 09:57:40 +0200

Roberto Maurizzi

robm71@pluspora.com

This is the only way to get the information in a properly normalized database, except for the HUGE detail that using LIMIT ... OFFSET .. is suicide.

With a smart RDBMS you should get only 20 record in this case... (so yeah, hi mysql) but in the case of page 999999, you'll get enough results to fill all those pages to then throw away 99998.

Usually there's a way to force or teach automatic admin pages (because I think that's what this is) to avoid using LIMIT ... OFFSET ..., they should research their docs on how to do that (that normally in your kind of situation gets changed in "YOU should research the docs, find the example that explains in detail why they're useless moron and send the link to them cc: their boss" ;-) )

Wed, 15 May 2019 10:08:45 +0200

Roberto Maurizzi

robm71@pluspora.com

If you know and can tell me which application engine they're using, assuming it's not a super-proprietary thing with paywalled docs, I'm sure we can find a documentation page that shows how to configure the queries to avoid "death by LIMIT .... OFFSET ..." so then you can stick it in their faces :D That said, as Jason Spieker said, I find it really strange that the RDBMS didn't "get" that it's only going to use 20 records linked to the first 20 records of the s_user table and limited the temp table building to those records only...