You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@fluo.apache.org by cjmctague <gi...@git.apache.org> on 2016/08/11 17:39:18 UTC
[GitHub] incubator-fluo pull request #760: Fixes #758
GitHub user cjmctague opened a pull request:
https://github.com/apache/incubator-fluo/pull/760
Fixes #758
* Modifed get(Collection<RowColumn> rowColumns) return type.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cjmctague/incubator-fluo fluo-758
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-fluo/pull/760.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #760
----
commit acc9a3d0e2af905d59ed70c892a5dac696435393
Author: Christopher McTague <cj...@vwc.edu>
Date: 2016-08-11T17:35:18Z
Fixes #758
* Modifed get(Collection<RowColumn> rowColumns) return type.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fluo issue #760: Fixes #758
Posted by cjmctague <gi...@git.apache.org>.
Github user cjmctague commented on the issue:
https://github.com/apache/incubator-fluo/pull/760
Going through to fix the `@Override` but this seem to be breaking a lot of other things.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fluo pull request #760: Fixes #758
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/incubator-fluo/pull/760#discussion_r74605858
--- Diff: modules/core/src/main/java/org/apache/fluo/core/impl/TransactionImpl.java ---
@@ -190,6 +190,8 @@ public Bytes get(Bytes row, Column column) {
return ret;
}
+ // TODO: Fix override to return Map<RowColumn, Bytes>
+ // TODO: Fix span() as this will move to unsupported type.
--- End diff --
Could create a new `HashMap<RowColumn, Bytes>` and loop over what pss.scan() returned adding to the new hash map. Could add an inner loop to exisitng loop.... like the following.
```java
//create HashMap<RowColumn, Bytes>
for (Entry<Bytes, Map<Column, Bytes>> entry : ret.entrySet()) {
updateColumnsRead(entry.getKey(), entry.getValue().keySet());
for (Entry<Column, Bytes> cols : entry.getValue().entrySet()) {
//create RowColumn and add to new map
}
}
//return new map
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fluo issue #760: Fixes #758
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the issue:
https://github.com/apache/incubator-fluo/pull/760
> For sure in way over my head on this..
Can you add something like `#TODO fix` everywhere there is a compile error? Then push that change. Then we can discuss those here in the PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fluo issue #760: Fixes #758
Posted by cjmctague <gi...@git.apache.org>.
Github user cjmctague commented on the issue:
https://github.com/apache/incubator-fluo/pull/760
For sure in way over my head on this..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fluo pull request #760: Fixes #758
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-fluo/pull/760
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fluo pull request #760: Fixes #758
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/incubator-fluo/pull/760#discussion_r74587856
--- Diff: modules/api/src/main/java/org/apache/fluo/api/client/SnapshotBase.java ---
@@ -51,10 +51,10 @@
Map<Bytes, Map<Column, Bytes>> get(Collection<Bytes> rows, Set<Column> columns);
/**
- * Given a collection of {@link RowColumn}s, retrieves a map contains the values at those rows and
- * {@link Column}s. Only rows and columns that exists will be returned in map.
+ * Given a collection of {@link RowColumn}s, retrieves a map contains the values at
+ * {@link RowColumn}. Only rows and columns that exists will be returned in map.
*/
- Map<Bytes, Map<Column, Bytes>> get(Collection<RowColumn> rowColumns);
+ Map<RowColumn, Bytes> get(Collection<RowColumn> rowColumns);
--- End diff --
Also need to modify the `gets(Collection<RowColumn>)`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fluo pull request #760: Fixes #758
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/incubator-fluo/pull/760#discussion_r74606664
--- Diff: modules/core/src/main/java/org/apache/fluo/core/impl/TxStringUtil.java ---
@@ -56,6 +56,7 @@ public static String gets(SnapshotBase snapshot, String row, Column column) {
return transform(snapshot.get(Collections2.transform(rows, s -> Bytes.of(s)), columns));
}
+ // TODO: Fix incompatable return type.
--- End diff --
Are you familiar w/ Lambda's in Java8? Could replace method with the following, which uses `Maps.transformValues()` and passes it a Lambda to transform the value from Bytes to String.
```java
public static Map<RowColumn, String> gets(SnapshotBase snapshot, Collection<RowColumn> rowColumns) {
Map<RowColumn, Bytes> bytesMap = snapshot.get(rowColumns);
return Maps.transformValues(bytesMap, b -> b.toString());
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fluo issue #760: Fixes #758
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on the issue:
https://github.com/apache/incubator-fluo/pull/760
These change look good, I squashed and merged them. Thanks @cjmctague.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-fluo pull request #760: Fixes #758
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/incubator-fluo/pull/760#discussion_r74639628
--- Diff: modules/core/src/main/java/org/apache/fluo/core/impl/TransactionImpl.java ---
@@ -203,12 +203,17 @@ public Bytes get(Bytes row, Column column) {
ParallelSnapshotScanner pss = new ParallelSnapshotScanner(rowColumns, env, startTs, stats);
Map<Bytes, Map<Column, Bytes>> ret = pss.scan();
+ Map<RowColumn, Bytes> bet = new HashMap<>();
for (Entry<Bytes, Map<Column, Bytes>> entry : ret.entrySet()) {
updateColumnsRead(entry.getKey(), entry.getValue().keySet());
+ for (Entry<Column, Bytes> cols : entry.getValue().entrySet()) {
+ // create RowColumn and add to new map
+ bet.put(new RowColumn(cols.getValue()), cols.getValue());
--- End diff --
`cols` should probably be called `colVal`. You need the row and col. The entry in the outer loop has the row. The entry in the inner loop has col and value.
```java
for (Entry<Bytes, Map<Column, Bytes>> entry : ret.entrySet()) {
updateColumnsRead(entry.getKey(), entry.getValue().keySet());
for (Entry<Column, Bytes> colVal : entry.getValue().entrySet()) {
Bytes row = entry.getKey();
Column col = colVal.getKey();
Bytes val = colVal.getValue();
bet.put(new RowColumn(row, col), val);
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---