sumitagrawl commented on code in PR #7167:
URL: https://github.com/apache/ozone/pull/7167#discussion_r1749625223
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -570,7 +625,74 @@ public Void call() {
return null;
}
- Map<String, Object> getFilteredObject(Object obj, Class<?> clazz,
Map<String, Object> fieldsSplitMap) {
+ boolean checkFilteredObject(Object obj, Class<?> clazz, Map<String,
Object> fieldsSplitMap)
+ throws IOException {
+ for (Map.Entry<String, Object> field : fieldsSplitMap.entrySet()) {
+ try {
+ Field valueClassField = getRequiredFieldFromAllFields(clazz,
field.getKey());
+ Object valueObject = valueClassField.get(obj);
+ Object fieldValue = field.getValue();
+
+ if (fieldValue == null) {
+ throw new IOException("Malformed filter. Check input");
+ } else if (fieldValue instanceof Filter) {
+ Filter filter = (Filter)fieldValue;
+ // reached the end of fields hierarchy, check if they match the
filter
+ // Currently, only equals operation is supported
+ if (Filter.FilterOperator.EQUALS.equals(filter.getOperator()) &&
+ !String.valueOf(valueObject).equals(filter.getValue())) {
+ return false;
+ } else if
(!Filter.FilterOperator.EQUALS.equals(filter.getOperator())) {
+ throw new IOException("Only EQUALS operator is supported
currently.");
+ }
+ } else {
+ Map<String, Object> subfields = (Map<String, Object>)fieldValue;
+ if (Collection.class.isAssignableFrom(valueObject.getClass())) {
+ if (!checkFilteredObjectCollection((Collection) valueObject,
subfields)) {
Review Comment:
if valueObject has null value, i.e. value is option or not present, in that
case, need ignore that object. Otherwise further access can cause
NullPointerException
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -570,7 +625,74 @@ public Void call() {
return null;
}
- Map<String, Object> getFilteredObject(Object obj, Class<?> clazz,
Map<String, Object> fieldsSplitMap) {
+ boolean checkFilteredObject(Object obj, Class<?> clazz, Map<String,
Object> fieldsSplitMap)
+ throws IOException {
+ for (Map.Entry<String, Object> field : fieldsSplitMap.entrySet()) {
+ try {
+ Field valueClassField = getRequiredFieldFromAllFields(clazz,
field.getKey());
+ Object valueObject = valueClassField.get(obj);
+ Object fieldValue = field.getValue();
+
+ if (fieldValue == null) {
+ throw new IOException("Malformed filter. Check input");
+ } else if (fieldValue instanceof Filter) {
+ Filter filter = (Filter)fieldValue;
+ // reached the end of fields hierarchy, check if they match the
filter
+ // Currently, only equals operation is supported
+ if (Filter.FilterOperator.EQUALS.equals(filter.getOperator()) &&
+ !String.valueOf(valueObject).equals(filter.getValue())) {
+ return false;
+ } else if
(!Filter.FilterOperator.EQUALS.equals(filter.getOperator())) {
+ throw new IOException("Only EQUALS operator is supported
currently.");
+ }
+ } else {
+ Map<String, Object> subfields = (Map<String, Object>)fieldValue;
+ if (Collection.class.isAssignableFrom(valueObject.getClass())) {
+ if (!checkFilteredObjectCollection((Collection) valueObject,
subfields)) {
+ return false;
+ }
+ } else if (Map.class.isAssignableFrom(valueObject.getClass())) {
+ Map<?, ?> valueObjectMap = (Map<?, ?>) valueObject;
+ for (Map.Entry<?, ?> ob : valueObjectMap.entrySet()) {
+ boolean flag;
+ if
(Collection.class.isAssignableFrom(ob.getValue().getClass())) {
+ flag =
checkFilteredObjectCollection((Collection)ob.getValue(), subfields);
+ } else {
+ flag = checkFilteredObject(ob.getValue(),
ob.getValue().getClass(), subfields);
+ }
+ if (flag) {
Review Comment:
When specific hierarchy matches, need check other object, so need continue
instead of return true. This is in assumption that it can store,
a.b.c.d:equal:20 and p.q.r.s:equal:30
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -504,6 +516,24 @@ Map<String, Object> getFieldSplit(List<String> fields,
Map<String, Object> field
return fieldMap;
}
+ Map<String, Object> getFilterSplit(List<String> fields, Map<String,
Object> fieldMap, Filter value) {
+ int len = fields.size();
+ if (fieldMap == null) {
+ fieldMap = new HashMap<>();
+ }
+ if (len == 1) {
+ fieldMap.putIfAbsent(fields.get(0), value);
+ } else {
+ Map<String, Object> fieldMapGet = (Map<String, Object>)
fieldMap.get(fields.get(0));
+ if (fieldMapGet == null) {
+ fieldMap.put(fields.get(0), getFilterSplit(fields.subList(1, len),
null, value));
Review Comment:
As code understanding, it may cause overwrite if prefix field is same for 2
different full key path, eg:
a.b.c.d:equals:10,a.b.m.n:equal:40
Also, in other patterns, this may have some overwrite of other filters.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DBScanner.java:
##########
@@ -570,7 +625,74 @@ public Void call() {
return null;
}
- Map<String, Object> getFilteredObject(Object obj, Class<?> clazz,
Map<String, Object> fieldsSplitMap) {
+ boolean checkFilteredObject(Object obj, Class<?> clazz, Map<String,
Object> fieldsSplitMap)
+ throws IOException {
+ for (Map.Entry<String, Object> field : fieldsSplitMap.entrySet()) {
+ try {
+ Field valueClassField = getRequiredFieldFromAllFields(clazz,
field.getKey());
+ Object valueObject = valueClassField.get(obj);
+ Object fieldValue = field.getValue();
+
+ if (fieldValue == null) {
+ throw new IOException("Malformed filter. Check input");
+ } else if (fieldValue instanceof Filter) {
+ Filter filter = (Filter)fieldValue;
+ // reached the end of fields hierarchy, check if they match the
filter
+ // Currently, only equals operation is supported
+ if (Filter.FilterOperator.EQUALS.equals(filter.getOperator()) &&
+ !String.valueOf(valueObject).equals(filter.getValue())) {
+ return false;
+ } else if
(!Filter.FilterOperator.EQUALS.equals(filter.getOperator())) {
+ throw new IOException("Only EQUALS operator is supported
currently.");
+ }
+ } else {
+ Map<String, Object> subfields = (Map<String, Object>)fieldValue;
+ if (Collection.class.isAssignableFrom(valueObject.getClass())) {
+ if (!checkFilteredObjectCollection((Collection) valueObject,
subfields)) {
+ return false;
+ }
+ } else if (Map.class.isAssignableFrom(valueObject.getClass())) {
+ Map<?, ?> valueObjectMap = (Map<?, ?>) valueObject;
+ for (Map.Entry<?, ?> ob : valueObjectMap.entrySet()) {
+ boolean flag;
+ if
(Collection.class.isAssignableFrom(ob.getValue().getClass())) {
+ flag =
checkFilteredObjectCollection((Collection)ob.getValue(), subfields);
+ } else {
+ flag = checkFilteredObject(ob.getValue(),
ob.getValue().getClass(), subfields);
+ }
+ if (flag) {
+ return true;
+ }
+ }
+ return false;
+ } else {
+ if (!checkFilteredObject(valueObject, valueClassField.getType(),
subfields)) {
+ return false;
+ }
+ }
+ }
+ } catch (NoSuchFieldException ex) {
+ err().println("ERROR: no such field: " + field);
Review Comment:
If not able to verify, we should avoid considering that record, in these
case, need return false in exception case,
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
