[ 
https://issues.apache.org/jira/browse/HBASE-15616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15686690#comment-15686690
 ] 

Jianwei Cui commented on HBASE-15616:
-------------------------------------

Sorry to reply late [~anoop.hbase], and thanks for your questions.

bq. So we can do this way of not setting qualifier on PB when qualifier is 
null? Do we need pass empty string to be set?
The mentioned code is defined in AccessControlUtil.java? The 
{{permissionBuilder}} is the builder of proto message {{TablePermission}}. The 
qualifier is optional in pb definition of TablePermission. When the qualifier 
is not setting, it has clear meaning as the permission is granted on any column 
of the family? On the other hand, checkAndMutate must check a specific column, 
so that qualifier field is required in pb definition of Condition: 
{code}
message Condition {
  required bytes row = 1;
  required bytes family = 2;
  required bytes qualifier = 3;
  ...
{code}
When the qualifier is null, it's also a legal column, I think pass empty string 
seems more clear in this situation?

bq. There are some other places in RequestConverter, we are doing this 
setQualifier(ByteStringer.wrap(qualifier))  See buildIncrementRequest() eg.
HTable provides two ways to do increment:
{code}
  public long incrementColumnValue(final byte [] row, final byte [] family, 
final byte [] qualifier, final long amount)
  public Result increment(final Increment increment)
{code}
The first method will check qualifier and throw NPE if qualifier is null before 
issuing request to server, so that we can't use the first method to do 
increment on null qualifier column. In the second method, {{Increment}} 
provides two ways to add a column:
{code}
public Increment addColumn(byte [] family, byte [] qualifier, long amount)
public Increment add(Cell cell)
{code}
{{addColumn}} will also check qualifier is not null, however {{add(Cell cell)}} 
won't do such check, so we can do increment on null qualifier column as:
{code}
      Increment incr = new Increment(Bytes.toBytes("row"));
      KeyValue kv = new KeyValue(Bytes.toBytes("row"), Bytes.toBytes("C"), 
null, HConstants.LATEST_TIMESTAMP, KeyValue.Type.Put, Bytes.toBytes(1l));
      incr.add(kv);
      table.increment(incr);
{code}
Therefore, the increment methods of HTable have different behaviors when 
qualifier is null, which looks confused. I think the null qualifier is legal in 
HBase, so that should be allowed in different increment methods, and we can 
also pass empty string as null qualifier in buildIncrementRequest()? What do 
you think [~anoop.hbase]? Thanks!

> CheckAndMutate will encouter NPE if qualifier to check is null
> --------------------------------------------------------------
>
>                 Key: HBASE-15616
>                 URL: https://issues.apache.org/jira/browse/HBASE-15616
>             Project: HBase
>          Issue Type: Bug
>          Components: Client
>    Affects Versions: 2.0.0
>            Reporter: Jianwei Cui
>            Assignee: Jianwei Cui
>         Attachments: HBASE-15616-v1.patch, HBASE-15616-v2.patch
>
>
> If qualifier to check is null, the checkAndMutate/checkAndPut/checkAndDelete 
> will encounter NPE.
> The test code:
> {code}
> table.checkAndPut(row, family, null, Bytes.toBytes(0), new 
> Put(row).addColumn(family, null, Bytes.toBytes(1)));
> {code}
> The exception:
> {code}
> Exception in thread "main" 
> org.apache.hadoop.hbase.client.RetriesExhaustedException: Failed after 
> attempts=3, exceptions:
> Fri Apr 08 15:51:31 CST 2016, 
> RpcRetryingCaller{globalStartTime=1460101891615, pause=100, maxAttempts=3}, 
> java.io.IOException: com.google.protobuf.ServiceException: 
> java.lang.NullPointerException
> Fri Apr 08 15:51:31 CST 2016, 
> RpcRetryingCaller{globalStartTime=1460101891615, pause=100, maxAttempts=3}, 
> java.io.IOException: com.google.protobuf.ServiceException: 
> java.lang.NullPointerException
> Fri Apr 08 15:51:32 CST 2016, 
> RpcRetryingCaller{globalStartTime=1460101891615, pause=100, maxAttempts=3}, 
> java.io.IOException: com.google.protobuf.ServiceException: 
> java.lang.NullPointerException
>       at 
> org.apache.hadoop.hbase.client.RpcRetryingCallerImpl.callWithRetries(RpcRetryingCallerImpl.java:120)
>       at org.apache.hadoop.hbase.client.HTable.checkAndPut(HTable.java:772)
>       at ...
> Caused by: java.io.IOException: com.google.protobuf.ServiceException: 
> java.lang.NullPointerException
>       at 
> org.apache.hadoop.hbase.protobuf.ProtobufUtil.getRemoteException(ProtobufUtil.java:341)
>       at org.apache.hadoop.hbase.client.HTable$7.call(HTable.java:768)
>       at org.apache.hadoop.hbase.client.HTable$7.call(HTable.java:755)
>       at 
> org.apache.hadoop.hbase.client.RpcRetryingCallerImpl.callWithRetries(RpcRetryingCallerImpl.java:99)
>       ... 2 more
> Caused by: com.google.protobuf.ServiceException: 
> java.lang.NullPointerException
>       at 
> org.apache.hadoop.hbase.ipc.AbstractRpcClient.callBlockingMethod(AbstractRpcClient.java:239)
>       at 
> org.apache.hadoop.hbase.ipc.AbstractRpcClient$BlockingRpcChannelImplementation.callBlockingMethod(AbstractRpcClient.java:331)
>       at 
> org.apache.hadoop.hbase.protobuf.generated.ClientProtos$ClientService$BlockingStub.mutate(ClientProtos.java:35252)
>       at org.apache.hadoop.hbase.client.HTable$7.call(HTable.java:765)
>       ... 4 more
> Caused by: java.lang.NullPointerException
>       at com.google.protobuf.LiteralByteString.size(LiteralByteString.java:76)
>       at 
> com.google.protobuf.CodedOutputStream.computeBytesSizeNoTag(CodedOutputStream.java:767)
>       at 
> com.google.protobuf.CodedOutputStream.computeBytesSize(CodedOutputStream.java:539)
>       at 
> org.apache.hadoop.hbase.protobuf.generated.ClientProtos$Condition.getSerializedSize(ClientProtos.java:7483)
>       at 
> com.google.protobuf.CodedOutputStream.computeMessageSizeNoTag(CodedOutputStream.java:749)
>       at 
> com.google.protobuf.CodedOutputStream.computeMessageSize(CodedOutputStream.java:530)
>       at 
> org.apache.hadoop.hbase.protobuf.generated.ClientProtos$MutateRequest.getSerializedSize(ClientProtos.java:12431)
>       at 
> org.apache.hadoop.hbase.ipc.IPCUtil.getTotalSizeWhenWrittenDelimited(IPCUtil.java:311)
>       at 
> org.apache.hadoop.hbase.ipc.AsyncRpcChannel.writeRequest(AsyncRpcChannel.java:409)
>       at 
> org.apache.hadoop.hbase.ipc.AsyncRpcChannel.callMethod(AsyncRpcChannel.java:333)
>       at 
> org.apache.hadoop.hbase.ipc.AsyncRpcClient.call(AsyncRpcClient.java:245)
>       at 
> org.apache.hadoop.hbase.ipc.AbstractRpcClient.callBlockingMethod(AbstractRpcClient.java:226)
>       ... 7 more
> {code}
> The reason is {{LiteralByteString.size()}} will throw NPE if wrapped byte 
> array is null. It is possible to invoke {{put}} and {{checkAndMutate}} on the 
> same column, because null qualifier is allowed for {{Put}},  users may be 
> confused if null qualifier is not allowed for {{checkAndMutate}}. We can also 
> convert null qualifier to empty byte array for {{checkAndMutate}} in client 
> side. Discussions and suggestions are welcomed. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to