You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/01/03 01:00:24 UTC
[GitHub] [drill] cgivre opened a new pull request #2132: DRILL-7831: Drill Fails To Read XML File with Self Closing Tags
cgivre opened a new pull request #2132:
URL: https://github.com/apache/drill/pull/2132
# [DRILL-7831](https://issues.apache.org/jira/browse/DRILL-7831): Drill Fails To Read XML File with Self Closing Tags
## Description
This PR addresses two issues with the XML plugin for Drill:
1. In cases where there were self closing tags, Drill was not reading them correctly. This PR fixes this.
2. Attribute names were not being parsed correctly in the case of addresses with underscores. This is reported in [DRILL-7832](https://issues.apache.org/jira/browse/DRILL-7832).
## Documentation
No user facing changes.
## Testing
Several new unit tests added to this PR.
----------------------------------------------------------------
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
[GitHub] [drill] cgivre merged pull request #2132: DRILL-7831: Drill Fails To Read XML File with Self Closing Tags
Posted by GitBox <gi...@apache.org>.
cgivre merged pull request #2132:
URL: https://github.com/apache/drill/pull/2132
----------------------------------------------------------------
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
[GitHub] [drill] cgivre commented on pull request #2132: DRILL-7831: Drill Fails To Read XML File with Self Closing Tags
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2132:
URL: https://github.com/apache/drill/pull/2132#issuecomment-755869325
@luocooong
I made an error when I rebased to the current master, and messed up a unit test. It has since been fixed. I will merge tomorrow.
----------------------------------------------------------------
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
[GitHub] [drill] cgivre commented on a change in pull request #2132: DRILL-7831: Drill Fails To Read XML File with Self Closing Tags
Posted by 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
[GitHub] [drill] luocooong commented on pull request #2132: DRILL-7831: Drill Fails To Read XML File with Self Closing Tags
Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2132:
URL: https://github.com/apache/drill/pull/2132#issuecomment-755416125
@cgivre +1
----------------------------------------------------------------
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
[GitHub] [drill] luocooong commented on a change in pull request #2132: DRILL-7831: Drill Fails To Read XML File with Self Closing Tags
Posted by GitBox <gi...@apache.org>.
luocooong commented on a change in pull request #2132:
URL: https://github.com/apache/drill/pull/2132#discussion_r551814647
##########
File path: contrib/format-xml/src/test/java/org/apache/drill/exec/store/xml/TestXMLReader.java
##########
@@ -83,6 +83,37 @@ public void testWildcard() throws Exception {
new RowSetComparison(expected).verifyAndClearAll(results);
}
+ @Test
+ public void testSelfClosingTags() throws Exception {
+ String sql = "SELECT * FROM cp.`xml/weather.xml`";
+ RowSet results = client.queryBuilder().sql(sql).rowSet();
+ assertEquals(1, results.rowCount());
+
+ TupleMetadata expectedSchema = new SchemaBuilder()
+ .addMap("attributes")
+ .addNullable("forecast_information_city_data", MinorType.VARCHAR)
+ .addNullable("forecast_information_postal_code_data", MinorType.VARCHAR)
+ .addNullable("forecast_information_latitude_e6_data", MinorType.VARCHAR)
+ .addNullable("forecast_information_longitude_e6_data", MinorType.VARCHAR)
+ .addNullable("forecast_information_forecast_date_data", MinorType.VARCHAR)
+ .addNullable("forecast_information_current_date_time_data", MinorType.VARCHAR)
+ .addNullable("forecast_information_unit_system_data", MinorType.VARCHAR)
+ .addNullable("forecast_information_current_conditions_condition_data", MinorType.VARCHAR)
+ .addNullable("forecast_information_current_conditions_temp_f_data", MinorType.VARCHAR)
+ .addNullable("forecast_information_current_conditions_temp_c_data", MinorType.VARCHAR)
+ .addNullable("forecast_information_current_conditions_humidity_data", MinorType.VARCHAR)
+ .addNullable("forecast_information_current_conditions_icon_data", MinorType.VARCHAR)
+ .addNullable("forecast_information_current_conditions_wind_condition_data", MinorType.VARCHAR)
Review comment:
This is the power of Drill. Any data reading can be like unpacking a complex parcel.
----------------------------------------------------------------
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
[GitHub] [drill] cgivre merged pull request #2132: DRILL-7831: Drill Fails To Read XML File with Self Closing Tags
Posted by GitBox <gi...@apache.org>.
cgivre merged pull request #2132:
URL: https://github.com/apache/drill/pull/2132
----------------------------------------------------------------
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
[GitHub] [drill] luocooong commented on pull request #2132: DRILL-7831: Drill Fails To Read XML File with Self Closing Tags
Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2132:
URL: https://github.com/apache/drill/pull/2132#issuecomment-754533464
I used these code to simply test the Chinese characters and it works fine.
----------------------------------------------------------------
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
[GitHub] [drill] luocooong commented on a change in pull request #2132: DRILL-7831: Drill Fails To Read XML File with Self Closing Tags
Posted by GitBox <gi...@apache.org>.
luocooong commented on a change in pull request #2132:
URL: https://github.com/apache/drill/pull/2132#discussion_r551759800
##########
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()` ?
----------------------------------------------------------------
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