bitoffdev commented on a change in pull request #1959:
URL: https://github.com/apache/hbase/pull/1959#discussion_r444922986
##########
File path: hbase-shell/src/main/ruby/hbase/admin.rb
##########
@@ -701,40 +703,40 @@ def alter(table_name_str, wait = true, *args)
# Delete column family
if method == 'delete'
raise(ArgumentError, 'NAME parameter missing for delete method')
unless name
- htd.removeFamily(name.to_java_bytes)
+ tdb.removeColumnFamily(name.to_java_bytes)
hasTableUpdate = true
# Unset table attributes
elsif method == 'table_att_unset'
raise(ArgumentError, 'NAME parameter missing for table_att_unset
method') unless name
if name.is_a?(Array)
name.each do |key|
- if htd.getValue(key).nil?
+ if tdb.build.getValue(key).nil?
raise ArgumentError, "Could not find attribute: #{key}"
end
- htd.remove(key)
+ tdb.setValue(key, nil)
Review comment:
Unfortunately, the current overloads for
`TableDescriptorBuilder.removeValue` take either `Bytes`, or `byte[]`, whereas
`setValue` has an overload for a String argument. **Unless there is an
objection, I will go ahead and add the `removeValue(final String key)`
overload**, which would be consistent with the `setValue` overloads, and may be
helpful for other developers in the future as well.
The other alternative (converting the key to bytes) seems like it would be
more effort than just using `setValue` as is.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
