Hussain Towaileb has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/3370 )
Change subject: [ASTERIXDB-2535][COMP] Fix uuid present in insert/upsert statement ...................................................................... Patch Set 5: (3 comments) https://asterix-gerrit.ics.uci.edu/#/c/3370/4/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordMergeEvaluator.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordMergeEvaluator.java: https://asterix-gerrit.ics.uci.edu/#/c/3370/4/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordMergeEvaluator.java@162 PS4, Line 162: handleDuplicateFieldNameButNotOfTypeRecord() > We should work on a better name for the method. Can we start by formulating What would you recommend that I rename this method to? The behavior of the normal record-merge is as follows: if 2 fields have the same name: - Check if they have the same value: - - Yes: continue, and take the left field only (identical fields). - - No: - - - If they're both of type Record, then do sub-record merge. - - - If they're not of type Record, throw duplicate field exception. To handle the issue of the uuid, at the point the 2 fields are equal in name, it means the primary key field is present. In case their value is not the same, the following behavior is done: - If they're both of type Record, then do sub-record merge. - If they're not of type Record, do NOT throw an exception, and instead take the left field only, because the left field is actually the primary key that was provided by the insert/upsert statement. I didn't want to make lots of changes in the existing code, but I could add the above documentation to it, is that what you're looking for? https://asterix-gerrit.ics.uci.edu/#/c/3370/4/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordMergeEvaluator.java@207 PS4, Line 207: IVisitablePointable leftValue, IVisitablePointable rightValue, boolean openFromParent, int nestedLevel) > As this is a private method, moving the parameter should be feasible and sa As this is part of the normal record-merge behavior, I thought it would be best to leave it untouched to focus this change only on the uuid issue. I can make a small follow up change for this part. https://asterix-gerrit.ics.uci.edu/#/c/3370/4/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordMergeUuidIgnoreDuplicateEvaluator.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordMergeUuidIgnoreDuplicateEvaluator.java: https://asterix-gerrit.ics.uci.edu/#/c/3370/4/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordMergeUuidIgnoreDuplicateEvaluator.java@37 PS4, Line 37: > do we need this? this function should always ignore uuid duplicates. Done, removed. You're right, that was from when the logic was hardcoded inside the old merge function, we don't need it now with this function. -- To view, visit https://asterix-gerrit.ics.uci.edu/3370 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I22100d3ff29864b8bfd54b0decb183e5056fdb4a Gerrit-Change-Number: 3370 Gerrit-PatchSet: 5 Gerrit-Owner: Hussain Towaileb <hussai...@gmail.com> Gerrit-Reviewer: Ali Alsuliman <ali.al.solai...@gmail.com> Gerrit-Reviewer: Anon. E. Moose (1000171) Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Gerrit-Reviewer: Hussain Towaileb <hussai...@gmail.com> Gerrit-Reviewer: Hussain Towaileb <hussai...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Comment-Date: Tue, 07 May 2019 09:45:43 +0000 Gerrit-HasComments: Yes
