[jira] [Comment Edited] (ACCUMULO-4746) Create Builder for Mutation
[
https://issues.apache.org/jira/browse/ACCUMULO-4746?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16274743#comment-16274743
]
Benjamin F edited comment on ACCUMULO-4746 at 12/7/17 1:27 PM:
---
I like the idea of a Fluent interface in this context. It works well since we
certainly can't set everything in the constructor and lends well to creating
mutations; fluency comes from making a new value from an old one which we
always do with mutations. Plus this covers your use case of the caller just
being able to chain overloaded methods such as
{{someMutation.mutate().family(foo)}} without having too much concern if foo is
a String or byte[].
My only (very small) concern is how well it will be able to flow semantically.
I think changing the name of the {{mutate()}} method as you mentioned in the
code comment could solve this, having the action word mutate near the front of
the chain seems like an action is being taken before I as the caller have set
the values. This is the only potential change I could think of but I think the
'new' term in my function name could be misleading and doesn't necessarily
match up with what's in the {{Mutation}} class:
{code:java}
m1.newRowChange()
.family(fam)
.qualifier("q1")
.set("v1")
{code}
Using {{newRowChange}} matches up with the verbiage in the Accumulo book
(pg.29, Introduction to the Client API) where the {{Mutation}} object is
described as a set of changes applied to a single row. Having {{set()}} be the
only action word in the chain makes it more apparent as an end to the chain.
Overall I think this is a great direction to go.
was (Author: bmfach):
I like the idea of a Fluent interface in this context. It works well since we
certainly can't set everything in the constructor and lends well to creating
mutations; fluency comes from making a new value from an old one which we
always do with mutations. Plus this covers your use case of the caller just
being able to chain overloaded methods such as
`someMutation.mutate().family(foo)` without having too much concern if `foo` is
a String or byte[].
My only (very small) concern is how well it will be able to flow semantically.
I think changing the name of the `mutate` method as you mentioned in the code
comment could solve this, having the action word mutate near the front of the
chain seems like an action is being taken before I as the caller have set the
values. This is the only potential change I could think of but I think the
'new' term in my function name could be misleading and doesn't necessarily
match up with what's in the `Mutation` class:
`m1.newRowChange()
.family(fam)
.qualifier("q1")
.set("v1")`
Using `newRowChange` matches up with the verbiage in the Accumulo book (pg.29,
Introduction to the Client API) where the `Mutation` object is described as a
set of changes applied to a single row. Having `set()` be the only action word
in the chain makes it more apparent as an end to the chain. Overall I think
this is a great direction to go.
> Create Builder for Mutation
> ---
>
> Key: ACCUMULO-4746
> URL: https://issues.apache.org/jira/browse/ACCUMULO-4746
> Project: Accumulo
> Issue Type: Sub-task
> Components: client
>Reporter: Keith Turner
>Assignee: Benjamin F
> Fix For: 2.0.0
>
>
> Accumulo needs a builder for mutation similar to the one that was added for
> Key.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Comment Edited] (ACCUMULO-4746) Create Builder for Mutation
[
https://issues.apache.org/jira/browse/ACCUMULO-4746?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16280640#comment-16280640
]
Keith Turner edited comment on ACCUMULO-4746 at 12/6/17 6:38 PM:
-
Another entry point function to consider is {{for()}}. What I like about
{{for()}} is that it reads a bit like a sentence (e.g. For the family X and
qualifier Y set the value to Z.)
{code:java}
m1.for().family(fam).qualifier("q1").set("v1");
m1.for().family(fam).qualifier("q2").set(bb);
m1.for().family(fam).qualifier("q3").delete();
{code}
I think I like {{at()}} more than {{for()}}.
was (Author: kturner):
Another entry point function to consider is `for()`. What I like about `for()`
is that it reads a bit like a sentence (e.g. For the family X and qualifier Y
set the value to Z.)
{code:java}
m1.for().family(fam).qualifier("q1").set("v1");
m1.for().family(fam).qualifier("q2").set(bb);
m1.for().family(fam).qualifier("q3").delete();
{code}
I think I like `at()` more than `for()`.
> Create Builder for Mutation
> ---
>
> Key: ACCUMULO-4746
> URL: https://issues.apache.org/jira/browse/ACCUMULO-4746
> Project: Accumulo
> Issue Type: Sub-task
> Components: client
>Reporter: Keith Turner
>Assignee: Benjamin F
> Fix For: 2.0.0
>
>
> Accumulo needs a builder for mutation similar to the one that was added for
> Key.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Comment Edited] (ACCUMULO-4746) Create Builder for Mutation
[
https://issues.apache.org/jira/browse/ACCUMULO-4746?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16269423#comment-16269423
]
Keith Turner edited comment on ACCUMULO-4746 at 11/28/17 8:43 PM:
--
While trying to puzzle through the issues I was running into with
ConditionalMutation I thought about this issue more deeply. I came to the
conclusion that a builder is not the right path for Mutation. For Key it made
more sense because Key had a lot of constructors and no setters. However for
mutation, it has all of these put methods. So after creating a mutation with a
builder, its possible to call put methods to further change it. This seems
silly to me.
What is nice about the key builder is that it allows easy mixing of different
types. For example its easy to have a byte array for the row, String for the
family, and byte array for the qualifier. Currently this is not possible with
the mutation put methods. There are put methods for all byte arrays or all
strings, but not combinations.
Based on the reasons above, I think having a fluent way to mix types for an
existing mutation instead of a builder is a better path. If we were designing
the API from scratch I think a builder that creates immutable mutations would
be the way to go. However that is not an option for the current API. Below is
a proposed Fluent API for mutating that easily supports mixing types.
{code:java}
class Mutation {
public interface FamilyStep extends QualifierStep {
// TODO other types?
QualifierStep family(CharSequence f);
QualifierStep family(byte[] f);
QualifierStep family(ByteBuffer f);
}
public interface QualifierStep extends VisibilityStep {
// TODO other types?
VisibilityStep qualifier(CharSequence q);
VisibilityStep qualifier(byte[] q);
VisibilityStep qualifier(ByteBuffer q);
}
public interface VisibilityStep extends TimestampStep {
// TODO other types?
TimestampStep visibility(CharSequence cv);
TimestampStep visibility(byte[] cv);
TimestampStep visibility(ByteBuffer cv);
TimestampStep visibility(ColumnVisibility cv);
}
public interface TimestampStep extends MutateStep {
MutateStep timestamp(long t);
}
public interface MutateStep {
// TODO other types?
void set(byte[] v);
void set(ByteBuffer v);
void set(CharSequence v);
void delete();
}
// this could be called put...
public FamilyStep mutate() {
//TODO implement.. could return a private inner class that implements all
interfaces...
return null;
}
}
{code}
Below is an example of using the new API that shows easily mixing different
types
{code:java}
Mutation m1 = new Mutation("row1");
byte[] fam = new byte[] {42, 21, 7, 9, 3, 34};
ByteBuffer bb = ...;
m1.mutate().family(fam).qualifier("q1").set("v1");
m1.mutate().family(fam).qualifier("q2").set(bb);
m1.mutate().family(fam).qualifier("q3").delete();
ByteBuffer bb2 = ...;
//below is an example of using the existing put method with different types
byte[] tmp = new byte[bb2.remaining()];
bb2.put(tmp);
// must make all types same, so made all into byte arrays
m1.put(fam, "q4".getBytes(StandardCharsets.UTF_8), tmp);
{code}
was (Author: kturner):
While trying to puzzle through the issues I was running into with
ConditionalMutation I thought about this issue more deeply. I came to the
conclusion that a builder is not the right path for Mutation. For Key it made
more sense because Key had a lot of constructors and no setters. However for
mutation, it has all of these put methods. So after creating a mutation with a
builder, its possible to call put methods to further change it. This seems
silly to me.
What is nice about the key builder is that it allows easy mixing of different
types. For example its easy to have a byte array for the row, String for the
family, and byte array for the qualifier. Currently this is not possible with
the mutation put methods. There are put methods for all byte arrays or all
strings, but not combinations.
Based on the reasons above, I think having a fluent way to mix types for an
existing mutation instead of a builder is a better path. If we were designing
the API from scratch I think a builder that creates immutable mutations would
be the way to go. However that is not an option for the current API. Below is
a proposed Fluent API for mutating that easily supports mixing types.
{code:java}
class Mutation {
public interface FamilyStep extends QualifierStep {
// TODO other types?
QualifierStep family(CharSequence f);
QualifierStep family(byte[] f);
QualifierStep family(ByteBuffer f);
}
public interface QualifierStep extends VisibilityStep {
// TODO other types?
VisibilityStep qualifier(CharSequence q);
VisibilityStep qualifier(byte[]
[jira] [Comment Edited] (ACCUMULO-4746) Create Builder for Mutation
[
https://issues.apache.org/jira/browse/ACCUMULO-4746?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16269132#comment-16269132
]
Keith Turner edited comment on ACCUMULO-4746 at 11/28/17 5:47 PM:
--
I was experimenting with an API for this builder and came up with the following.
{code:java}
class Mutation {
public interface Builder extends QualifierStep {
// TODO other types
QualifierStep family(CharSequence f);
public Mutation build();
}
public interface QualifierStep extends VisibilityStep {
// TODO other types
VisibilityStep qualifier(CharSequence q);
}
public interface VisibilityStep extends TimestampStep {
// TODO other types
TimestampStep visibility(CharSequence v);
}
public interface TimestampStep extends MutateStep {
// TODO other types
MutateStep timestamp(long t);
}
public interface MutateStep {
Builder set(CharSequence v);
Builder delete();
}
//TODO support other types for row
public static Builder builder(CharSequence row) {
return null; // TODO
}
}
{code}
Using the API above, can write code like the following to create mutations.
{code:java}
Builder builder = Mutation.builder("row1");
builder.family("f1").qualifier("q1").set("v1");
builder.family("f1").qualifier("q2").delete();
builder.qualifier("q1").set("v2"); // empty family
Mutation m = builder.build();
Mutation m2 = Mutation.builder("row1")
.family("f1").qualifier("q1").set("v1")
.family("f1").qualifier("q2").set("v2")
.family("f1").qualifier("q3").delete()
.family("f2").visibility("A").set("v3") // empty qualifier
.family("f3").qualifier("q1").visibility("A|B").timestamp(56).set("v4").build();
}
{code}
was (Author: kturner):
I was experimenting with an API for this builder and came up with the following.
{code:java}
class Mutation {
public interface Builder extends QualifierStep {
// TODO other types
QualifierStep family(CharSequence f);
public Mutation build();
}
public interface QualifierStep extends VisibilityStep {
// TODO other types
VisibilityStep qualifier(CharSequence q);
}
public interface VisibilityStep extends TimestampStep {
// TODO other types
TimestampStep visibility(CharSequence v);
}
public interface TimestampStep extends MutateStep {
// TODO other types
MutateStep timestamp(long t);
}
public interface MutateStep {
Builder set(CharSequence v);
Builder delete();
}
//TODO support other types for row
public static Builder builder(CharSequence row) {
return null; // TODO
}
}
{code}
Using the API above, can write code like the following to create mutations.
{code:java}
Builder builder = Mutation.builder("row1");
builder.family("f1").qualifier("q1").set("v1");
builder.family("f1").qualifier("q2").delete();
builder.qualifier("q1").set("v2"); // empty qualifier
Mutation m = builder.build();
Mutation m2 = Mutation.builder("row1")
.family("f1").qualifier("q1").set("v1")
.family("f1").qualifier("q2").set("v2")
.family("f1").qualifier("q3").delete()
.family("f2").visibility("A").set("v3") // empty qualifier
.family("f3").qualifier("q1").visibility("A|B").timestamp(56).set("v4").build();
}
{code}
> Create Builder for Mutation
> ---
>
> Key: ACCUMULO-4746
> URL: https://issues.apache.org/jira/browse/ACCUMULO-4746
> Project: Accumulo
> Issue Type: Sub-task
> Components: client
>Reporter: Keith Turner
> Fix For: 2.0.0
>
>
> Accumulo needs a builder for mutation similar to the one that was added for
> Key.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
