joshelser commented on a change in pull request #3124:
URL: https://github.com/apache/hbase/pull/3124#discussion_r607291323
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ColumnFamilyDescriptorBuilder.java
##########
@@ -1376,5 +1404,16 @@ public ModifyableColumnFamilyDescriptor
setStoragePolicy(String policy) {
return setValue(STORAGE_POLICY_BYTES, policy);
}
+ /**
+ * Validates values of HBase defined column family attributes.
+ * Throws {@link IllegalArgumentException} if the value of attribute is
not proper format.
+ * @param key
+ * @param value
+ */
+ public void validateAttributeValue(Bytes key, Bytes value) {
+ if(ATTRIBUTE_VALIDATOR.containsKey(key)) {
+ ATTRIBUTE_VALIDATOR.get(key).apply(value);
Review comment:
Would be nice to catch the formatting exception here and give a clear
error as to what attribute failed to parse.
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ColumnFamilyDescriptorBuilder.java
##########
@@ -305,6 +308,30 @@
RESERVED_KEYWORDS.add(new Bytes(Bytes.toBytes(IS_MOB)));
RESERVED_KEYWORDS.add(new Bytes(Bytes.toBytes(MOB_THRESHOLD)));
RESERVED_KEYWORDS.add(new
Bytes(Bytes.toBytes(MOB_COMPACT_PARTITION_POLICY)));
+ initValidator();
+ }
+
+ private static void initValidator() {
+ ATTRIBUTE_VALIDATOR.put(DATA_BLOCK_ENCODING_BYTES,
+ (d) -> DataBlockEncoding.valueOf(d.toString()));
+ ATTRIBUTE_VALIDATOR.put(COMPRESSION_BYTES,
+ (ca) -> Compression.Algorithm.valueOf(ca.toString()));
+ ATTRIBUTE_VALIDATOR.put(BLOOMFILTER_BYTES, (bl) ->
BloomType.valueOf(bl.toString()));
+ ATTRIBUTE_VALIDATOR.put(REPLICATION_SCOPE_BYTES, (rs) ->
Integer.valueOf(rs.toString()));
+ ATTRIBUTE_VALIDATOR.put(MIN_VERSIONS_BYTES, (min) ->
Integer.valueOf(min.toString()));
+ ATTRIBUTE_VALIDATOR.put(MAX_VERSIONS_BYTES, (max) ->
Integer.valueOf(max.toString()));
+ ATTRIBUTE_VALIDATOR.put(BLOCKSIZE_BYTES, (bs) ->
Integer.valueOf(bs.toString()));
+ ATTRIBUTE_VALIDATOR.put(KEEP_DELETED_CELLS_BYTES,
+ (k) -> KeepDeletedCells.valueOf(k.toString()));
+ ATTRIBUTE_VALIDATOR.put(TTL_BYTES, (ttl) ->
Integer.valueOf(ttl.toString()));
+ ATTRIBUTE_VALIDATOR.put(DFS_REPLICATION_BYTES, (dr) ->
Integer.valueOf(dr.toString()));
+ ATTRIBUTE_VALIDATOR.put(MOB_THRESHOLD_BYTES, (mt) ->
Long.valueOf(mt.toString()));
+ ATTRIBUTE_VALIDATOR.put(MOB_COMPACT_PARTITION_POLICY_BYTES,
+ (mc) -> MobCompactPartitionPolicy.valueOf(mc.toString()));
+ ATTRIBUTE_VALIDATOR.put(IN_MEMORY_COMPACTION_BYTES,
+ (im) -> MemoryCompactionPolicy.valueOf(im.toString()));
+ ATTRIBUTE_VALIDATOR.put(COMPRESSION_COMPACT_BYTES,
+ (ca) -> Compression.Algorithm.valueOf(ca.toString()));
Review comment:
Doing the client-side validation is an improvement over what we have
today. Can we do a server-side validation check as well?
For example, if a client is using new libraries than the server does, and
(hypothetically) the client knows about some newer enum value that the server
doesn't. A client-side check would fail first.
Should be pretty straightforward to add a new step into CreateTableProcedure
and ModifyTableProcedure to check the HTD the user provides.
https://github.com/apache/hbase/blob/202b17f4fc3229e91d584837776d53ed3e2e8adb/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java#L280-L286
https://github.com/apache/hbase/blob/202b17f4fc3229e91d584837776d53ed3e2e8adb/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java#L287-L328
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ColumnFamilyDescriptorBuilder.java
##########
@@ -282,6 +283,8 @@
private final static Set<Bytes> RESERVED_KEYWORDS = new HashSet<>();
+ private final static Map<Bytes,Function<Bytes,Object>> ATTRIBUTE_VALIDATOR =
new HashMap<>();
Review comment:
If we want to do this same validation server-side, it would be nice to
life the map and the validator functions out to their own class which can be
more easily re-used.
--
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
