[
https://issues.apache.org/jira/browse/PHOENIX-1060?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14347716#comment-14347716
]
James Taylor commented on PHOENIX-1060:
---------------------------------------
Thanks for following up on this, [~rajeshbabu]. Here's some feedback on your
patch:
- Make familyPtr and qualifierPtr final instead of volatile:
{code}
diff --git
a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/covered/update/ColumnReference.java
b/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/covered/update/ColumnReference.java
index ae0421d..6ea06e6 100644
---
a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/covered/update/ColumnReference.java
+++
b/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/covered/update/ColumnReference.java
@@ -30,59 +30,50 @@ public class ColumnReference implements
Comparable<ColumnReference> {
public static final byte[] ALL_QUALIFIERS = new byte[0];
- private static int calcHashCode(byte[] family, byte[] qualifier) {
+ private static int calcHashCode(ImmutableBytesWritable familyPtr,
ImmutableBytesWritable qualifierPtr) {
final int prime = 31;
int result = 1;
- result = prime * result + Bytes.hashCode(family);
- result = prime * result + Bytes.hashCode(qualifier);
+ result = prime * result + familyPtr.hashCode();
+ result = prime * result + qualifierPtr.hashCode();
return result;
}
- private final int hashCode;
- protected final byte[] family;
- protected final byte[] qualifier;
- private volatile ImmutableBytesWritable familyPtr;
- private volatile ImmutableBytesWritable qualifierPtr;
+ private final int hashCode;
+ private volatile ImmutableBytesPtr familyPtr;
+ private volatile ImmutableBytesPtr qualifierPtr;
{code}
- To be safe, make all of the setter methods of the ImmutableBytesPtr throw so
that you guarantee that ColumnReference is immutable (as that's the intention).
Might be cleanest to just add a ReadOnlyImmutableBytesPtr as this might come up
again.
- We should cache the byte[] on demand if the byte[], offset, length
constructor is called, as otherwise the getFamily() and getQualifier() calls
will allocate a new byte[] for each call. An alternative might be to get rid of
those methods, but there are probably a lot of callers (though it's possible
they could all be converted to be ok with an ImmutableBytesWritable).
> Replace ReferencingColumn with ColumnReference
> ----------------------------------------------
>
> Key: PHOENIX-1060
> URL: https://issues.apache.org/jira/browse/PHOENIX-1060
> Project: Phoenix
> Issue Type: Sub-task
> Reporter: rajeshbabu
> Assignee: Rajeshbabu Chintaguntla
> Priority: Minor
> Fix For: 5.0.0, 4.3.1
>
> Attachments: PHOENIX-1060.patch
>
>
> We currently have two separate classes that represent a column family/column
> qualifier combo: ColumnReference and ReferencingColumn. I believe the only
> difference is that ReferencingColumn allows a backing byte array to be used
> with an offset and length. Since ColumnReference already has an
> ImmutableBytesPtr we can add a constructor that takes two offsets and lengths
> for the family and qualifier and do a bit of minor refactoring (removing the
> family and qualifier member variables and just use the familyPtr and
> qualifierPtr ones exclusivly) so that we don't have two classes for the same
> thing:
> {code}
> public class ColumnReference implements Comparable<ColumnReference> {
>
> public ColumnReference(byte[] family, int familyOffset, int familyLength,
> byte[] qualifier, int qualifierOffset, int qualiferLength) {
> this.familyPtr = new ImmutableBytesPtr(family, familyOffset,
> familyLength);
> this.qualifierPtr = new ImmutableBytesPtr(qualifier, qualifierOffset,
> qualifierLength);
> this.hashCode = calcHashCode(familyPtr.get(), qualifierPtr.get());
> }
> {code}
> Another useful addition would be to implement Writable so we don't duplicate
> the serialization logic.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
