You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by vj...@apache.org on 2021/01/29 15:28:33 UTC
[hbase] branch master updated: HBASE-25533 The metadata of the
table and family should not be an empty string (#2906)
This is an automated email from the ASF dual-hosted git repository.
vjasani pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new e877506 HBASE-25533 The metadata of the table and family should not be an empty string (#2906)
e877506 is described below
commit e8775060ddb6c7fbf05d3d2bedc28110f127cf06
Author: Baiqiang Zhao <zb...@gmail.com>
AuthorDate: Fri Jan 29 23:27:31 2021 +0800
HBASE-25533 The metadata of the table and family should not be an empty string (#2906)
Signed-off-by: Viraj Jasani <vj...@apache.org>
Signed-off-by: Geoffrey Jacoby <gj...@apache.org>
---
.../client/ColumnFamilyDescriptorBuilder.java | 4 +-
.../hbase/client/TableDescriptorBuilder.java | 6 +--
.../client/TestColumnFamilyDescriptorBuilder.java | 19 +++++++++
.../hbase/client/TestTableDescriptorBuilder.java | 18 +++++++++
hbase-shell/src/test/ruby/hbase/admin_test.rb | 47 ++++++++++++++++++++++
5 files changed, 89 insertions(+), 5 deletions(-)
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 9a47cb5..7afc387 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
@@ -677,7 +677,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);
@@ -1228,7 +1228,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 d983868..2581cce 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
@@ -701,7 +701,7 @@ public class TableDescriptorBuilder {
toBytesOrNull(value, Bytes::toBytes));
}
- /*
+ /**
* @param key The key.
* @param value The value. If null, removes the setting.
*/
@@ -710,14 +710,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 557d2f8..7528d24 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.assertThrows;
import static org.junit.Assert.assertTrue;
@@ -210,6 +211,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 43824af..05a0b31 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
@@ -369,4 +369,22 @@ public class TestTableDescriptorBuilder {
htd = TableDescriptorBuilder.newBuilder(htd).setRegionServerGroup(null).build();
assertNull(htd.getValue(RSGroupInfo.TABLE_DESC_PROP_GROUP));
}
+
+ @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 64a4a8b..309624a 100644
--- a/hbase-shell/src/test/ruby/hbase/admin_test.rb
+++ b/hbase-shell/src/test/ruby/hbase/admin_test.rb
@@ -1013,6 +1013,21 @@ 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 raise error trying to remove nonexistent attributes" do
drop_test_table(@test_name)
@@ -1064,6 +1079,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 "alter should raise error trying to remove nonexistent configurations" do
drop_test_table(@test_name)
create_test_table(@test_name)