- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
I actually HAVE read every TDWTF article, joke's on you! ...except I forgot to record the URL of the first one.
Admin
Well yes. What if you need to port this code to a database that doesn't support your stinking MIN and MAX functions? Hmm? There'll be tears before bedtime, I can assure you.
Admin
We're WAY out of line here. Missing context, the entire logic set, and gross assumptions about what he's up to. Perhaps he does other things with $resultmin later on. Perhaps displaytable does other things we can't see (it's got TWO extra Parms) in addition to building $resultmin which is poorly named, it's not $resultmin it's $resultALL. So $resultALL has all rows and he also needs just the ID's of the First and Last in variables for unknown reasons.
How ELSE you gonna do it, run a Second Query, Mr. I_Love_Unnecessary_IO?
I started 40 years ago and was taught the general principle that Memory is faster than IO. I don't think that has changed.
Admin
Unless you are planning to port it some simple key value store like berklydb or something I think you can pretty confident any SQL database will support MIN and MAX which are ANSI functions. You can reasonably argue a database that does not support MIN and MAX cannot be a SQL database. So I don't think there is a lot to worry about in terms of portability.
Admin
This code is not reading the min/max of a value but is instead getting the hmid of the first/last records in the recordset. And from the looks of the code the only way to determine the number of records in the record set is to iterate over it. Hence this code seems to be the best for what it does.
Admin
Admin
Erm...except the SQL orders by hmid. Therefore the first record will be the min value and the last record will be the max value.
Admin
D'oh, I should have had my coffee before I posted. However I also forgot that doing Min/Max together in a single query can be slower than doing them separately depending on how the DB handles indexes (google for that if you really care). So I'm going to stand by my statement that the person who wrote the original code knows what they are doing and has local domain knowledge as to why this code is the best way of doing things.
Admin
I'm going to assume the person who submitted this WTF has the same domain knowledge as the original coder, and that therefore the submitted code is dumb.
Admin
I'v seen a lot of strange code in my time and I know there is a fine line between brilliant and brillant. Without knowing the greater context you sometimes can't tell one from the other.
Admin
Any query which fetches Max and Min hmid will always be faster or at least equal (in case of same index scan in both queries) to any query which fetches all hmid which will also have extra network latency for additional rows of data to be returned not to mention CPU time used up in iterations. OP is most likely affected by NIH for min and max.
Admin
So you are postulating a person who knows that they want Min/Max, is capable of correctly crafting an SQL query that both sorts and has a 2 part where clause, yet is unaware of min/max functions?
Admin
No need to assume that at all. I worked at a company that didn't allow aggregate functions in SQL queries, needless to say I didn't work there very long.
Admin
You were right the first time: the result is ordered by "hmid", the min/max results are "trim(hmid)", so the first/last record need not be the min/maximum.
Admin
Admin
Sorry you didn't appreciate or at least get the accuracy and humor (I guess that makes us even :-). To answer your questions just in case they were not rhetorical: No and No.
Admin
Agreed.
The presumption of WTF is based on the assumption that all the code needs to do is get the min and max. But it's clear that that's not actually all that the code does. The code actually pulls back all records into a recordset and then gets the max and min. If a later piece of code not included in the current snippet uses the rest of the recordset, then it smarter to iterate on the recordset that you've already fetched instead of creating another query just to get the max and min.
Admin
It seems to me that SQL needs a WFT parser. One that yells out "W..T..F" loudly when it detects terrible constructs. It might solve many problems, which seem to appear in all sorts of SQL querys. Maybe TDWTF ought to put "keywords" in a cornet to reflect which class it is in. I suggest a few:
SQL (Obviously)
Regular expressions. (Now you have two problems!).
Bosses. (Usually of the pointy haired kind).
Excel. (It grew like slime).
...The list goes on.....
Admin
This is job security code.
Ticket (after 3 months): Sorting is too slow. Solution: Removed applicable data. Ticket (after 4 months): Sorting is too slow. Solution: No data to remove, put $count_rcds = $count_rcds + ($counter - 1); at end of loop, trim max at end of loop. Ticket (after 5 months): Sorting is too slow. Solution: Removed applicable data. Ticket (after 7 months): Sorting is too slow. Solution: Removed order by, use comparison for min, max. Ticket (after 10 months): Sorting is too slow. Solution: Take loop out of displayTable and put into foreach instead of intermediate table. Ticket (after 12 months): Sorting is too slow. Solution: Removed applicable data. Ticket (after 14 months): Sorting is too slow. Solution: Used MIN/MAX in mysql. Ticket (after 17 months): Sorting is too slow. Solution: Removed applicable data. Ticket (after 19 months): Sorting is too slow. Solution: Add index to hmid. Ticket (after 19 months): Sorting is broke. Solution: Trimmed all data in database. Ticket (after 20 months): Sorting is broke for new entries. Solution: Trimmed all data in database, trim added on user input. Ticket (after 22 months): Sorting is too slow. Solution: Removed applicable data. Ticket (after 24 months): Sorting is too slow. Solution: Added index to tag_order_ref. Ticket (after 26 months): Sorting is too slow. Solution: Added index to tagrefgrp to index.
And that's only the half of it. Missing at some point head and tail of the array, etc.
Always make sure you never truly fix things if you can. Every thing must have as many future potential problems as possible. Make sure they know they depend on you.
Admin
You can't use an index to sort on trimmed data usually. Sometimes databases sort of self trim which is annoying. Assuming that isn't the problem, the code here is actually broke.
This one needs a three way index and either data validation or sanitisation to remain fast in this particular case. On the other hand the cardinality isn't really clear.
Admin
Admin
"Hey Q, I've raised 16 tickets on this one piece of code in the last 18 months -- hell no, that's 28 tickets in the last 2 years. I smell something fishy. Care to take a look at it quickly? Yes I know it's not your area, it's Wally's -- do you mind? Can you just check he's not featherbedding?"
Admin
This still heavily depends on an unknown implementation of 'order by' in the DB. Better would be to copy the result of an unsorted query into an Array and implement inhouse a reliable sort algorithm (lazy sort comes into mind).
Admin
Great idea! You should get right to doing it yourself. We can always enjoy a few more submissions here at TheDailyWTF™.
Admin
Ahh: so you consider MIN(TRIM(x)) to be the same as TRIM(MIN(x)) , which would make post-processing (indeed) fine. Did you ever ask yourself if MIN and TRIM commute? (hint: they don't)
Admin
Trim(Max) is wtf. Trim is a string function, and last time I checked, you can't trim numbers. Oh, silly me, those pesky numbers are stored in a text field, which makes it all so much easier. What sort of fool stores numbers in a numeric field, honestly, tsk tsk, and as for just putting a trim constraint on the field and eliminating the problem once and for all, well, that beggars belief.
Admin
[quote=Weizenflocke]This still heavily depends on an unknown implementation of 'order by' in the DB.[/quote]
:wtf: Of course we cannot trust a database to sort records, not when we have highly-paid engineers who can make sure that records come out in a configurable order tailored to the mission-critical demands of the enterprise.
Oddly enough, though, the use of trim() suggests that the field might be a string and won't sort the way we'd want in all cases.
But in spite of Remy's mockery, the joke's on him. The OP also computes the number of records in the result set -- a chore the makers of SQL never had the foresight to accommodate.
Addendum 2016-06-29 19:51: Curse you, lack of preview capability.
Admin
There's lots of reasons this could be slower, but...
This is what the SQL does functionally. The database engine might take shortcuts with indexes for MIN(hmid). But for MIN(TRIM(hmid)) the entire set of TRIM(hmid) would have to be materialized and sorted internally by the database engine.
Admin
If for some reason you need to do it then you do it when you have to. Doing stuff just in case it might be needed later costs a lot of time money. If you don't know what changes there will be (if any) don't try to be too smart beforehand.
Admin
Perhaps this is a mis-interpretation of "don't push business logic into the DB"?
If your experience with SQL like languages is Cassandra CQL prior to version 3 then there weren't aggregate functions.
Just a word to the wise. When a vendor claims their Big Data supports ANSI Standard SQL always ask which one. Quite often they means ANSI SQL-92. We've moved on quite a bit from that in the past 24 years!
Admin
This style is often seen in enterprisy cloudy software like salesforce where aggregate queries are so dumbed down, it is easier to select everything and loop through the results.
Admin
'Perhaps this is a mis-interpretation of "don't push business logic into the DB"?'
It would be a pretty straightforward extrapolation of it, since "don't push business logic into the dbms" is essentially saying, "don't put business logic in the tool specifically designed to represent business logic." If you accept that premise, it's pretty reasonable to go ahead and not use other features it provides.
Admin
Honestly what horrifies me the most is "grphdr_refgrp"... WHAT?! Whenever I see mangled variable names like that I feel like slapping someone in the face for each letter that she wanted to save.