bitoffdev commented on a change in pull request #1959:
URL: https://github.com/apache/hbase/pull/1959#discussion_r445790978
##########
File path: hbase-shell/src/main/ruby/hbase/admin.rb
##########
@@ -971,101 +976,103 @@ def enabled?(table_name)
end
#----------------------------------------------------------------------------------------------
- # Return a new HColumnDescriptor made of passed args
- def hcd(arg, htd)
+ # Return a new ColumnFamilyDescriptor made of passed args
+ def hcd(arg, tdb)
# String arg, single parameter constructor
- return org.apache.hadoop.hbase.HColumnDescriptor.new(arg) if
arg.is_a?(String)
+
+ return ColumnFamilyDescriptorBuilder.of(arg) if arg.is_a?(String)
raise(ArgumentError, "Column family #{arg} must have a name") unless
name = arg.delete(NAME)
- family = htd.getFamily(name.to_java_bytes)
+ cfd = tdb.build.getColumnFamily(name.to_java_bytes)
Review comment:
I think this is the best way to do it at the moment. Currently, it seems
that TableDescriptorBuilder is intended to be used for writing only, in which
case we would not want the builder itself to have methods like getColumnFamily
or hasColumnFamily. This is consistent with the lack of getValue and hasValue
methods on the builder.
Other than adding methods to builder, the only other way to shortcut the
handful of calls this patch makes to `tdb.build` would be to cache the
TableDescriptor at the start of each method that uses a T.D.. I have to
recommend against this approach since it would technically change the behavior.
Ex: If you were to execute something in the shell like `alter 't1', {NAME =>
'fam1', METHOD => 'delete'}, {NAME => 'fam1', VERSIONS => 5}` where a C.F. is
changed multiple times. In this example, a cached T.D. would not reflect the
deletion of the C.F..
**With these thoughts, I am inclined to leave the patch 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
