Query to determine insert / update operations to synchronize data

Question

I've been developing a system where the mobile syncs data from the server.The phone can run offline but when connected to the internet it needs take care of inserted or updated data.

Here is the output result (this is just for demonstration purposes it will be changed a bit):

[
 {
 "version_send": "1",
 "action_peformed": "insert,update",
 "version": "1,4",
 "change_date": "2017-06-22 16:42:03",
 "audit_name": "Push Ups",
 "current_name": "Pushup",
 "id_exercise": "1",
 "action_peformed_": "update"
 },
 {
 "version_send": "1",
 "action_peformed": "insert",
 "version": "1",
 "change_date": "2017-06-22 16:42:06",
 "audit_name": "Squat",
 "current_name": "Squat",
 "id_exercise": "2",
 "action_peformed_": "igonre"
 },
 {
 "version_send": "1",
 "action_peformed": "insert",
 "version": "1",
 "change_date": "2017-06-22 16:42:09",
 "audit_name": "Chin Ups",
 "current_name": "Chin Ups",
 "id_exercise": "3",
 "action_peformed_": "igonre"
 },
 {
 "version_send": "1",
 "action_peformed": "insert,update",
 "version": "2,3",
 "change_date": "2017-06-22 16:44:25",
 "audit_name": "Pull Ups",
 "current_name": "Pull Up",
 "id_exercise": "4",
 "action_peformed_": "insert"
 },
 {
 "version_send": "1",
 "action_peformed": "insert",
 "version": "2",
 "change_date": "2017-06-22 16:45:08",
 "audit_name": "Sit Up",
 "current_name": "Sit Up",
 "id_exercise": "5",
 "action_peformed_": "insert"
 },
 {
 "version_send": "1",
 "action_peformed": "insert,update,update",
 "version": "2,3,4",
 "change_date": "2017-06-22 16:45:28",
 "audit_name": "Pike Push Up",
 "current_name": "Pike Pushups",
 "id_exercise": "6",
 "action_peformed_": "insert"
 }
]

The goal here was to determine if the phones version if for update or for insert of the new data.

For example:

{
 "version_send": "1",
 "action_peformed": "insert,update,update",
 "version": "2,3,4",
 "change_date": "2017-06-22 16:45:28",
 "audit_name": "Pike Push Up",
 "current_name": "Pike Pushups",
 "id_exercise": "6",
 "action_peformed_": "insert"
 }

The data was "action_peformed": "insert,update,update" but the phone has the version 1, and the first insert on that data was from version two so the output action be insert.

But if the phone version was 2 it means that the action should be insert.. You get the point.

The current query that does this looks like this:

SELECT 
 :version as `version_send`,
 GROUP_CONCAT(ea.`action_peformed` ORDER BY ea.`action_peformed` ASC) as `action_peformed`,
 GROUP_CONCAT(ea.`version` ORDER BY ea.`version` ASC) as `version`,
 ea.change_date,
 ea.name as audit_name,
 e.name as current_name,
 e.id_exercise ,
 ##BEGIN Determine what should be done
 IF (:version < MAX(ea.`version`),
 IF(
 FIND_IN_SET (:version, 
 GROUP_CONCAT( DISTINCT ea.`version` SEPARATOR ',' ) 
 ), 
 'update',##'is less than max but has one or more versions - update',
 IF (:version > MIN(ea.`version`),
 'update',##'is less than max but has one or more versions - update',
 'insert'##'is less than max but has no versions - insert'
 )
 )
 ,'igonre') as `action_peformed_`
 ##END Determine what should be done*/
 FROM exercise_audit AS ea LEFT JOIN exercise AS e ON e.id_exercise = ea.reference_id GROUP BY e.id_exercise

And here are the tables that i used:

CREATE TABLE `exercise` (
 `id_exercise` int(11) NOT NULL,
 `name` varchar(45) NOT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
CREATE TABLE `exercise_audit` (
 `id_exercise_audit` int(11) NOT NULL,
 `name` varchar(45) NOT NULL COMMENT 'The name will take the new value if inserted ,or the old value if edited.And the latest value if deleted.',
 `action_peformed` varchar(45) NOT NULL,
 `version` int(11) NOT NULL,
 `reference_id` int(11) NOT NULL,
 `change_date` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
CREATE TABLE `version` (
 `id_version` int(11) NOT NULL,
 `date` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
 `state` enum('C','P') NOT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

Showing rows 0 - 5 (6 total, Query took 0.0010 seconds.)

I feel that the query is missing something, how could i improve it?

I someone is interested i can share the code over github, if it will be easier.

• 200_success: Are you aware that "igonre" is a misspelling?
• DaAmidza: Yes i have changed it in my code, but forgot to change it on the question.

Answer 1

Documentation

The data was "action_peformed": "insert,update,update" but the phone has the version 1, and the first insert on that data was from version two so the output action be insert.

But if the phone version was 2 it means that the action should be insert.. You get the point.

No, I'm afraid I don't get the point. Is version_send the phone's version? If so, am I supposed to be able to guess that from the name? And how could the phone have version 1 if the earliest existing version is 2?

---

Readability

Layout

The FROM is indented as far as the columns to select, and the GROUP BY is off screen unless I scroll horizontally. Styles vary, but if your style guide suggests hiding clauses as important as FROM and GROUP BY then I'd look for a different one. I personally prefer to indent both at the same level as SELECT, and to put each JOIN from the FROM clause on a separate line indented once.

There's also some inconsistent whitespace around commas and parentheses.

Comments

You seem to have changed comment style at some point, because there's one line

 ##END Determine what should be done*/

That's confusing. Using ## when the comment delimiter is just # also makes me wonder whether there's something odd going on, and that's not the kind of question you want a maintenance programmer to be asking.

Spelling

200_success already pointed out in comments that igonre is incorrect, but so is peformed. If you don't have backward compatibility constraints which prevent fixing it, I suggest fixing it.

Capitalisation

Most of the SQL keywords and MySQL built-ins are capitalised, but not all. In particular, why as rather than AS?

---

Logic

 GROUP_CONCAT(ea.`action_peformed` ORDER BY ea.`action_peformed` ASC) as `action_peformed`,
 GROUP_CONCAT(ea.`version` ORDER BY ea.`version` ASC) as `version`,

From what I did understand of the explanation as to how the data is used client-side, the versions and the actions should be in the same order. But if they are then it's pure coincidence. In particular, insert sorts alphabetically before update, but delete comes before either of them, so if the actions are an insert, an update, and a delete then action_performed will be in a different order to version. This looks like a major bug.

---

 ea.change_date,
 ea.name as audit_name,
 e.name as current_name,
 e.id_exercise ,

To my surprise (as someone who has used MySQL but first learnt SQL with a different database), this is actually legal, but comes with a caveat:

However, this is useful primarily when all values in each nonaggregated column not named in the GROUP BY are the same for each group. The server is free to choose any value from each group, so unless they are the same, the values chosen are indeterminate.

Going purely on my guesses, I would expect audit_name and possibly id_exercise to be the same for all exercise_audits in the group, but I would be astonished if change_date is and the name current_name suggests that that also changes. I suspect that here there are some more important bugs.

---

 FIND_IN_SET (:version, 
 GROUP_CONCAT( DISTINCT ea.`version` SEPARATOR ',' ) 
 ),

Firstly, I'm surprised to see DISTINCT. Does that mean that two audits can have the same version? If so, that looks like another source of bugs.

Secondly, is this really the best way to do that test? I'm not saying that I know a better one, but I would sincerely hope that there is one.

---

 IF (:version < MAX(ea.`version`),
 IF(
 FIND_IN_SET (:version, 
 GROUP_CONCAT( DISTINCT ea.`version` SEPARATOR ',' ) 
 ), 
 'update',##'is less than max but has one or more versions - update',
 IF (:version > MIN(ea.`version`),
 'update',##'is less than max but has one or more versions - update',
 'insert'##'is less than max but has no versions - insert'
 )
 )
 ,'igonre') as `action_peformed_`

So if :version is between MIN(ea.version) and MAX(ea.version) but is not found in the table, that's an update? It sounds to me like an error.

However, if the logic given here is correct, then we can refactor as

 IF (:version < MAX(ea.`version`),
 IF(
 :version > MIN(ea.`version`) OR FIND_IN_SET (:version, 
 GROUP_CONCAT( DISTINCT ea.`version` SEPARATOR ',' ) 
 ),
 'update',##'is less than max but has one or more versions - update',
 'insert'##'is less than max but has no versions - insert'
 )
 ,'igonre') as `action_peformed_`

and then observe that if :version is in the set, it's either greater than the least element in the set or equal to the least element in the set, so I think it's valid to refactor further as

 IF (:version < MAX(ea.`version`),
 IF(
 :version >= MIN(ea.`version`),
 'update',##'is less than max but has one or more versions - update',
 'insert'##'is less than max but has no versions - insert'
 )
 ,'igonre') as `action_peformed_`

The comments don't make much sense ("has one or more versions" is true if the group is non-empty, and if the group is empty then I don't think the expression would be evaluated at all), but that's a separate issue.

• DaAmidza: AS i have wrote at the beginning this was for testing purposes to explain the problem behind it.There is no DELETE in this system.Most of the things that you have mentioned that are considered as bugs are actually not.You can have from version n to x , and the phone will only have one version.Than it will request the missing versions and it will obtain them as i have described in my answer.The comments "has one or more versions" have more sense than you think.And yea my mistake since i wrote it quickly but 2 days ago i have changed the comments and the AS that you mentioned.
• DaAmidza: Your answer is good but it didn't help me out in any way to be honest.
• Peter Taylor: I have two responses to that: firstly, things which look like bugs are either bugs or things which need better comments. If they're not bugs, that's a sign that the comments need improving. Secondly, if you have an improved version, you're welcome to post a new question for that to be reviewed.
• DaAmidza: I accepted your answer since no one else answered and you have spent some time writing this.