ihuzenko commented on a change in pull request #1955: DRILL-7491: Incorrect
count() returned for complex types in parquet
URL: https://github.com/apache/drill/pull/1955#discussion_r366812967
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractGroupScanWithMetadata.java
##########
@@ -167,29 +167,46 @@ public boolean isMatchAllMetadata() {
*/
@Override
public long getColumnValueCount(SchemaPath column) {
- long tableRowCount, colNulls;
- Long nulls;
ColumnStatistics<?> columnStats =
getTableMetadata().getColumnStatistics(column);
- ColumnStatistics<?> nonInterestingColStats = null;
- if (columnStats == null) {
- nonInterestingColStats =
getNonInterestingColumnsMetadata().getColumnStatistics(column);
- }
+ ColumnStatistics<?> nonInterestingColStats = columnStats == null
+ ? getNonInterestingColumnsMetadata().getColumnStatistics(column) :
null;
+ long tableRowCount;
if (columnStats != null) {
tableRowCount =
TableStatisticsKind.ROW_COUNT.getValue(getTableMetadata());
} else if (nonInterestingColStats != null) {
tableRowCount =
TableStatisticsKind.ROW_COUNT.getValue(getNonInterestingColumnsMetadata());
Review comment:
Hello @paul-rogers ,
As says Javadoc for this method, here we're looking for a count of non-null
row values of the specific column. There is a special physical rule
```ConvertCountToDirectScanPrule``` purpose which is to reduce IO when we're
looking for ```count(col)``` values, since the value may be present in the
metadata read while planning. The problem which I'm trying to fix (
[DRILL-7491](https://issues.apache.org/jira/browse/DRILL-7491)) appeared in the
method because the method for existing complex column returned **0** as if the
column doesn't exist. So physical plan was converted to something like:
```
ScreenPrel
ProjectPrel(cr=[$0])
DirectScanPrel(groupscan=[selectionRoot =
classpath:/parquet/tmphive/hive_alltypes.parquet, numFiles = 1,
usedMetadataSummaryFile = false, usedMetastore = false,
DynamicPojoRecordReader{records = [[0]]}])
```
and as a result incorrect count was returned.
In debug mode, I noticed that parquet didn't provide metadata for top-level
complex type column, but some metadata exists for nested fields, so I decided
to check the existence of metadata to determine whether such complex column
exists in parquet file and that scan is necessary to calculate correct row
count value.
Regarding your comments about the improvement of a metadata structure for
complex fields, I think here there is no some special magic in Drill. I
suppose that we're just reading & converting metadata from parquet into our
model classes. If you have some good ideas on how we can improve our meta,
could you please start a discussion at our mailing list to hear ideas of the
community? Since the topic is out of scope for this pull request.
Kind regards,
Igor
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
