[GitHub] [drill] cgivre commented on a change in pull request #2132: DRILL-7831: Drill Fails To Read XML File with Self Closing Tags

[GitBox <gi...@apache.org>]

cgivre commented on a change in pull request #2132:
URL: https://github.com/apache/drill/pull/2132#discussion_r552621082



##########
File path: contrib/format-xml/src/main/java/org/apache/drill/exec/store/xml/XMLReader.java
##########
@@ -298,15 +305,27 @@ private void processEvent(XMLEvent currentEvent,
         } else if (currentState == xmlState.FIELD_ENDED && currentNestingLevel >= dataLevel) {
           // Case to end nested maps
           // Pop tupleWriter off stack
-          currentTupleWriter = rowWriterStack.pop();
-          attributePrefix = XMLUtils.removeField(attributePrefix);
+          if (rowWriterStack.size() > 0) {
+            currentTupleWriter = rowWriterStack.pop();
+          }
+          // Pop field name
+          if (fieldNameStack.size() > 0) {
+            fieldNameStack.pop();
+          }
+
+          attributePrefix = XMLUtils.removeField(attributePrefix,fieldName);

Review comment:
       > Could you please confirm that the `attributePrefix` variable will not be null in the context of the `XMLReader.processEvent()` ?
   
   @luocooong.  Thanks for the review.
   
   The field `attributePrefix` is defined here:
   
   https://github.com/apache/drill/blob/4e40104d5b4f6ef870f8569edc4981a9eb3ac7ca/contrib/format-xml/src/main/java/org/apache/drill/exec/store/xml/XMLReader.java#L247-L249
   
   If the field `currentNestingLevel` is greater than the `dataLevel` then it will be defined.  If not, then this entire code block will be skipped.  
   




----------------------------------------------------------------
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:
users@infra.apache.org