diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ColumnFamilyDescriptorBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ColumnFamilyDescriptorBuilder.java index e845ee9d233..cf7812ba544 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ColumnFamilyDescriptorBuilder.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ColumnFamilyDescriptorBuilder.java @@ -679,7 +679,7 @@ public class ColumnFamilyDescriptorBuilder { * @return this (for chained invocation) */ private ModifyableColumnFamilyDescriptor setValue(Bytes key, Bytes value) { - if (value == null) { + if (value == null || value.getLength() == 0) { values.remove(key); } else { values.put(key, value); @@ -1241,7 +1241,7 @@ public class ColumnFamilyDescriptorBuilder { * @return this (for chained invocation) */ public ModifyableColumnFamilyDescriptor setConfiguration(String key, String value) { - if (value == null) { + if (value == null || value.length() == 0) { configuration.remove(key); } else { configuration.put(key, value); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java index d35dd2c35af..53703211979 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java @@ -713,7 +713,7 @@ public class TableDescriptorBuilder { toBytesOrNull(value, Bytes::toBytes)); } - /* + /** * @param key The key. * @param value The value. If null, removes the setting. */ @@ -722,14 +722,14 @@ public class TableDescriptorBuilder { return setValue(key, toBytesOrNull(value, Bytes::toBytes)); } - /* + /** * Setter for storing metadata as a (key, value) pair in {@link #values} map * * @param key The key. * @param value The value. If null, removes the setting. */ public ModifyableTableDescriptor setValue(final Bytes key, final Bytes value) { - if (value == null) { + if (value == null || value.getLength() == 0) { values.remove(key); } else { values.put(key, value); diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestColumnFamilyDescriptorBuilder.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestColumnFamilyDescriptorBuilder.java index f3052292477..478bc0f8ae7 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestColumnFamilyDescriptorBuilder.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestColumnFamilyDescriptorBuilder.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.client; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import org.apache.hadoop.hbase.HBaseClassTestRule; @@ -222,6 +223,24 @@ public class TestColumnFamilyDescriptorBuilder { KeepDeletedCells.FALSE.toString()); assertEquals(defaultValueMap.get(ColumnFamilyDescriptorBuilder.DATA_BLOCK_ENCODING), DataBlockEncoding.NONE.toString()); + } + @Test + public void testSetEmptyValue() { + ColumnFamilyDescriptorBuilder builder = + ColumnFamilyDescriptorBuilder.newBuilder(HConstants.CATALOG_FAMILY); + String testConf = "TestConfiguration"; + String testValue = "TestValue"; + // test set value + builder.setValue(testValue, "2"); + assertEquals("2", Bytes.toString(builder.build().getValue(Bytes.toBytes(testValue)))); + builder.setValue(testValue, ""); + assertNull(builder.build().getValue(Bytes.toBytes(testValue))); + + // test set configuration + builder.setConfiguration(testConf, "1"); + assertEquals("1", builder.build().getConfigurationValue(testConf)); + builder.setConfiguration(testConf, ""); + assertNull(builder.build().getConfigurationValue(testConf)); } } diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java index 19f872193b8..7328194a93b 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.client; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -327,4 +328,22 @@ public class TestTableDescriptorBuilder { "{TABLE_ATTRIBUTES => {DURABILITY => 'ASYNC_WAL'}}, {NAME => 'cf', BLOCKSIZE => '1000'}", htd.toStringCustomizedValues()); } + + @Test + public void testSetEmptyValue() { + TableDescriptorBuilder builder = + TableDescriptorBuilder.newBuilder(TableName.valueOf(name.getMethodName())); + String testValue = "TestValue"; + // test setValue + builder.setValue(testValue, "2"); + assertEquals("2", builder.build().getValue(testValue)); + builder.setValue(testValue, ""); + assertNull(builder.build().getValue(Bytes.toBytes(testValue))); + + // test setFlushPolicyClassName + builder.setFlushPolicyClassName("class"); + assertEquals("class", builder.build().getFlushPolicyClassName()); + builder.setFlushPolicyClassName(""); + assertNull(builder.build().getFlushPolicyClassName()); + } } diff --git a/hbase-shell/src/test/ruby/hbase/admin_test.rb b/hbase-shell/src/test/ruby/hbase/admin_test.rb index 698a73b10d3..534a9d7064d 100644 --- a/hbase-shell/src/test/ruby/hbase/admin_test.rb +++ b/hbase-shell/src/test/ruby/hbase/admin_test.rb @@ -980,6 +980,22 @@ module Hbase assert_no_match(eval("/" + key_2 + "/"), admin.describe(@test_name)) end + define_test "alter should be able to remove a list of table attributes when value is empty" do + drop_test_table(@test_name) + + key_1 = "TestAttr1" + key_2 = "TestAttr2" + command(:create, @test_name, { NAME => 'i'}, METADATA => { key_1 => 1, key_2 => 2 }) + + # eval() is used to convert a string to regex + assert_match(eval("/" + key_1 + "/"), admin.describe(@test_name)) + assert_match(eval("/" + key_2 + "/"), admin.describe(@test_name)) + + command(:alter, @test_name, METADATA => { key_1 => '', key_2 => '' }) + assert_no_match(eval("/" + key_1 + "/"), admin.describe(@test_name)) + assert_no_match(eval("/" + key_2 + "/"), admin.describe(@test_name)) + end + define_test "alter should be able to remove a table configuration" do drop_test_table(@test_name) create_test_table(@test_name) @@ -1010,6 +1026,38 @@ module Hbase assert_no_match(eval("/" + key_2 + "/"), admin.describe(@test_name)) end + define_test "alter should be able to remove a list of table configuration when value is empty" do + drop_test_table(@test_name) + + key_1 = "TestConf1" + key_2 = "TestConf2" + command(:create, @test_name, { NAME => 'i'}, CONFIGURATION => { key_1 => 1, key_2 => 2 }) + + # eval() is used to convert a string to regex + assert_match(eval("/" + key_1 + "/"), admin.describe(@test_name)) + assert_match(eval("/" + key_2 + "/"), admin.describe(@test_name)) + + command(:alter, @test_name, CONFIGURATION => { key_1 => '', key_2 => '' }) + assert_no_match(eval("/" + key_1 + "/"), admin.describe(@test_name)) + assert_no_match(eval("/" + key_2 + "/"), admin.describe(@test_name)) + end + + define_test "alter should be able to remove a list of column family configuration when value is empty" do + drop_test_table(@test_name) + + key_1 = "TestConf1" + key_2 = "TestConf2" + command(:create, @test_name, { NAME => 'i', CONFIGURATION => { key_1 => 1, key_2 => 2 }}) + + # eval() is used to convert a string to regex + assert_match(eval("/" + key_1 + "/"), admin.describe(@test_name)) + assert_match(eval("/" + key_2 + "/"), admin.describe(@test_name)) + + command(:alter, @test_name, { NAME => 'i', CONFIGURATION => { key_1 => '', key_2 => '' }}) + assert_no_match(eval("/" + key_1 + "/"), admin.describe(@test_name)) + assert_no_match(eval("/" + key_2 + "/"), admin.describe(@test_name)) + end + define_test "get_table should get a real table" do drop_test_table(@test_name) create_test_table(@test_name)