[GROUND-23] De-relationalize Cassandra schema #72

seanlobo · 2017-04-14T20:30:46Z

I encapsulated some tables via Cassandra set and map collections. This uses fewer reads when accessing the data, which should be best practice.

vsreekanti · 2017-04-14T20:44:50Z

The Travis CI build failed. @seanlobo, you can see the failing tests here.

vsreekanti · 2017-04-14T20:47:55Z

scripts/cassandra/cassandra.cql

@@ -21,9 +21,12 @@ create table version (
 create table version_successor (
    id bigint PRIMARY KEY,
    from_version_id bigint,
-    to_version_id bigint
+    to_version_id bigint,
+    item_id_set set<bigint>


Why is this a set? Each version successor should only be a part of one Item's DAG. The VersionHistoryDag table was meant to be for quick access, but I'm not convinced that that's the case now that I think about it.

Regardless, there should just be one Item id per VersionSuccessor.

The item_id_set has been updated to to store a single item_id

vsreekanti · 2017-04-14T20:52:22Z

src/main/java/edu/berkeley/ground/dao/models/cassandra/CassandraStructureVersionFactory.java


 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;

-import org.slf4j.Logger;


These imports should stay in alphabetical order.

vsreekanti · 2017-04-14T20:54:12Z

src/main/java/edu/berkeley/ground/dao/usage/cassandra/CassandraLineageGraphVersionFactory.java

-        lineageEdgeVersionIds.add(lineageEdgeSet.getLong("lineage_edge_version_id"));
-      } while (lineageEdgeSet.next());
-    }
+    List<Long> lineageEdgeVersionIds = resultSet.getSet("lineage_edge_version_id_set", Long.class).stream().collect(Collectors.toList());


Can't you create a new List from a Set instead of streaming? See this constructor.

Updated to use the constructor

vsreekanti · 2017-04-14T20:56:01Z

src/main/java/edu/berkeley/ground/db/CassandraClient.java

@@ -192,6 +276,63 @@ public void delete(List<DbDataContainer> predicates, String table) {
    this.session.execute(statement);
  }

+	/**


Formatting.

vsreekanti · 2017-04-14T20:56:26Z

src/main/java/edu/berkeley/ground/db/CassandraClient.java

@@ -192,6 +276,63 @@ public void delete(List<DbDataContainer> predicates, String table) {
    this.session.execute(statement);
  }

+	/**
+   * Deletes a column from a table (sets column to null)


Deleting a column from a table sounds very different from changing it to null. I think setColumnValueToNull might be a more apt name?

The documentation was a mistake, the method does actually delete columns. The docs (and method) has been updated

vsreekanti · 2017-04-14T20:56:42Z

src/main/java/edu/berkeley/ground/db/CassandraClient.java

+    this.session.execute(statement);
+  }
+
+	/**


Formatting.

kevinji · 2017-04-14T21:30:34Z

src/main/java/edu/berkeley/ground/db/CassandraClient.java

@@ -84,6 +87,65 @@ public void insert(String table, List<DbDataContainer> insertValues) {
    this.session.execute(statement);
  }

+	/**


Too indented.

kevinji · 2017-04-14T21:32:24Z

src/main/java/edu/berkeley/ground/dao/models/cassandra/CassandraGraphVersionFactory.java

-      edgeInsertion.add(new DbDataContainer("edge_version_id", GroundType.LONG, edgeVersionId));
-
-      this.dbClient.insert("graph_version_edge", edgeInsertion);
+      Set<Long> edge = new HashSet<Long>();


I think HashSet<> is enough.

kevinji · 2017-04-14T21:33:06Z

src/main/java/edu/berkeley/ground/dao/models/cassandra/CassandraGraphVersionFactory.java

-        edgeVersionIds.add(edgeSet.getLong("edge_version_id"));
-      } while (edgeSet.next());
-    }
+    List<Long> edgeVersionIds = resultSet.getSet("edge_version_set", Long.class).stream().collect(Collectors.toList());


Same comment as Vikram's; new ArrayList<>(set) should be sufficient.

Updated to use the ArrayList constructor

Move table structure_version_attribute into map column of structure_version

Move graph_version_edge into set column of graph_version (edge_version_set)

Move lineage_graph_version_edge into lineage_graph_version (with set lineage_edge_version_id_set)

change item_id_set to a singular item_id

codecov-io · 2017-05-03T06:08:10Z

Codecov Report

Merging #72 into master will decrease coverage by 0.15%.
The diff coverage is 85.84%.

@@             Coverage Diff              @@
##             master      #72      +/-   ##
============================================
- Coverage      85.1%   84.94%   -0.16%     
- Complexity      626      637      +11     
============================================
  Files           115      115              
  Lines          3027     3083      +56     
  Branches        223      222       -1     
============================================
+ Hits           2576     2619      +43     
- Misses          405      419      +14     
+ Partials         46       45       -1

Impacted Files	Coverage Δ	Complexity Δ
...cassandra/CassandraLineageGraphVersionFactory.java	`100% <100%> (+2.43%)`	`5 <0> (-1)`	⬇️
...ns/cassandra/CassandraVersionSuccessorFactory.java	`94% <100%> (+1.14%)`	`10 <3> (+3)`	⬆️
...models/cassandra/CassandraGraphVersionFactory.java	`100% <100%> (+2.7%)`	`5 <0> (-1)`	⬇️
...s/cassandra/CassandraVersionHistoryDagFactory.java	`100% <100%> (ø)`	`12 <1> (-2)`	⬇️
app/db/CassandraResults.java	`81.48% <33.33%> (-13.76%)`	`13 <2> (+2)`
app/db/CassandraClient.java	`88.61% <80.35%> (-6.91%)`	`21 <6> (+10)`
...ls/cassandra/CassandraStructureVersionFactory.java	`94.59% <90.9%> (-0.41%)`	`6 <0> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1f5130...a2ea3a8. Read the comment docs.

now accepts a list of columns to delete

vsreekanti · 2017-05-22T21:01:04Z

Will merge this after porting the Cassandra schema over.

vsreekanti requested changes Apr 14, 2017

View reviewed changes

kevinji reviewed Apr 14, 2017

View reviewed changes

seanlobo added 13 commits April 20, 2017 17:08

Update Primary Keys to ensure uniqueness

516be54

Add methods to query and update sets from tables

eaf5b8c

Encapsulate Cassandra version_successor_dag into version_successor

86c63ae

Document set methods in CassandraClient

6cff1d0

Make all tests pass

543f47c

Add methods to update/delete/query Cassandra Maps/Sets

751393f

Encapsulate Cassandra structure_version_attribute

3679d11

Move table structure_version_attribute into map column of structure_version

Encapsulate Cassandra graph_version_edge

02f494f

Move graph_version_edge into set column of graph_version (edge_version_set)

Encapsulate Cassandra lineage_graph_version_edge

b3b6bd7

Move lineage_graph_version_edge into lineage_graph_version (with set lineage_edge_version_id_set)

Add method to delete from Cassandra set

7c50b4a

Fix Style and make tests pass

812eecd

Fix CassandraClient to pass upstream changes

1a0acce

Modify Cassandra version_successor table

af28a05

change item_id_set to a singular item_id

seanlobo force-pushed the cassandra-schema branch from 3be7de3 to af28a05 Compare May 3, 2017 05:49

Update CassandraClient.deleteColumn

a2ea3a8

now accepts a list of columns to delete

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GROUND-23] De-relationalize Cassandra schema #72

[GROUND-23] De-relationalize Cassandra schema #72

seanlobo commented Apr 14, 2017

vsreekanti commented Apr 14, 2017

vsreekanti Apr 14, 2017

seanlobo May 3, 2017

vsreekanti Apr 14, 2017

vsreekanti Apr 14, 2017

seanlobo May 3, 2017

vsreekanti Apr 14, 2017

seanlobo May 3, 2017

vsreekanti Apr 14, 2017

seanlobo May 3, 2017

vsreekanti Apr 14, 2017

seanlobo May 3, 2017

kevinji Apr 14, 2017

seanlobo May 3, 2017

kevinji Apr 14, 2017

seanlobo May 3, 2017

kevinji Apr 14, 2017 •

edited

Loading

seanlobo May 3, 2017

codecov-io commented May 3, 2017 •

edited

Loading

vsreekanti commented May 22, 2017

[GROUND-23] De-relationalize Cassandra schema #72

Are you sure you want to change the base?

[GROUND-23] De-relationalize Cassandra schema #72

Conversation

seanlobo commented Apr 14, 2017

vsreekanti commented Apr 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinji Apr 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented May 3, 2017 • edited Loading

Codecov Report

vsreekanti commented May 22, 2017

kevinji Apr 14, 2017 •

edited

Loading

codecov-io commented May 3, 2017 •

edited

Loading