ctubbsii commented on code in PR #1123:
URL: https://github.com/apache/fluo/pull/1123#discussion_r1024056223


##########
modules/integration-tests/src/main/java/org/apache/fluo/integration/impl/FailureIT.java:
##########
@@ -652,4 +655,40 @@ private boolean wasRolledBackPrimary(long startTs, String 
rolledBackRow)
     return sawExpected;
   }
 
+  /*
+   * There was a bug where a locked column with an empty family could not be 
recovered.
+   */
+  @Test
+  public void testRecoverEmptyColumn() {
+    Column ecol = new Column("", "bal");
+
+    TestTransaction tx1 = new TestTransaction(env);
+
+    tx1.set("bob", ecol, "10");
+    tx1.set("joe", ecol, "20");
+    tx1.set("jill", ecol, "60");
+
+    tx1.done();
+
+    // partially commit a transaction, leaving the row 'joe' with a lock.
+    TestTransaction tx2 = new TestTransaction(env);
+    TestUtil.increment(tx2, "bob", ecol, 5);
+    TestUtil.increment(tx2, "joe", ecol, -5);
+
+    CommitData cd = tx2.createCommitData();
+    RowColumn primary = new RowColumn("bob", ecol);
+    Assert.assertTrue(tx2.preCommit(cd, primary));
+    Stamp commitTs = env.getSharedResources().getOracleClient().getStamp();
+    tx2.commitPrimaryColumn(cd, commitTs);
+    tx2.close();
+
+    // this transaction should roll forward the above transaction
+    TestTransaction tx3 = new TestTransaction(env);
+    Assert.assertEquals("15", tx3.gets("bob", ecol));
+    Assert.assertEquals("15", tx3.gets("joe", ecol));
+    Assert.assertEquals("60", tx3.gets("jill", ecol));
+    tx3.close();

Review Comment:
   Tests could use try with resources to close the transactions



##########
modules/api/src/main/java/org/apache/fluo/api/data/Column.java:
##########
@@ -28,26 +28,42 @@
 public final class Column implements Comparable<Column>, Serializable {
 
   private static final long serialVersionUID = 1L;
-  private static final Bytes UNSET = Bytes.of(new byte[0]);
 
-  private Bytes family = UNSET;
-  private Bytes qualifier = UNSET;
-  private Bytes visibility = UNSET;
+  private final Bytes family;
+  private final Bytes qualifier;
+  private final Bytes visibility;
+
+  private final boolean isFamilySet;
+  private final boolean isQualifierSet;
+  private final boolean isVisibilitySet;

Review Comment:
   Instead of 6 fields, could use Optional to have only 3. Could use 
Optional.orElse(emptyBytes) in the getter instead of checking if the 
corresponding boolean is set



-- 
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.

To unsubscribe, e-mail: notifications-unsubscr...@fluo.apache.org

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

Reply via email to