[GitHub] [accumulo] keith-turner commented on a change in pull request #1637: #1618 Investigate TODO in exception message in new Bulk Import

[GitBox <gi...@apache.org>]

keith-turner commented on a change in pull request #1637:
URL: https://github.com/apache/accumulo/pull/1637#discussion_r451226815



##########
File path: core/src/main/java/org/apache/accumulo/core/clientImpl/bulk/BulkImport.java
##########
@@ -616,7 +616,9 @@ private Text toText(byte[] row) {
         if (containsPrevRow && containsEndRow) {
           mappings.get(ke).merge(mappings.remove(oke));
         } else {
-          throw new RuntimeException("TODO handle merges");
+          throw new RuntimeException("Unable to execute merge. Key extent "
+                  + oke.toString() + " extends beyond the range of Key extent "
+                  + ke.toString() + ". Out of range data would be lost.");

Review comment:
       @cradal  I was thinking this following code could be moved into a KeyExtent.contains(KeyExtent) method.  Maybe something like the following.
   
   ```java
   boolean contains(KeyExtent oke){
           boolean containsPrevRow = getPrevEndRow() == null || (oke.getPrevEndRow() != null
               && getPrevEndRow().compareTo(oke.getPrevEndRow()) <= 0);
           boolean containsEndRow = getEndRow() == null
               || (oke.getEndRow() != null && getEndRow().compareTo(oke.getEndRow()) >= 0);
   
          return containsPrevRow && containsEndRow;
   }
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org