You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by db...@apache.org on 2022/04/21 15:12:26 UTC
[impala] branch master updated: IMPALA-10839: NULL values are displayed on a wrong level for nested structs (ORC)
This is an automated email from the ASF dual-hosted git repository.
dbecker pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new c802be42b IMPALA-10839: NULL values are displayed on a wrong level for nested structs (ORC)
c802be42b is described below
commit c802be42b66cf69a362f7d2c89175932bbe226cf
Author: Daniel Becker <da...@cloudera.com>
AuthorDate: Tue Apr 12 16:44:15 2022 +0200
IMPALA-10839: NULL values are displayed on a wrong level for nested structs (ORC)
When querying a non-toplevel nested struct from an ORC file, the NULL
values are displayed at an incorrect level. E.g.:
select id, outer_struct.inner_struct3 from
functional_orc_def.complextypes_nested_structs where id >= 4;
+----+----------------------------+
| id | outer_struct.inner_struct3 |
+----+----------------------------+
| 4 | {"s":{"i":null,"s":null}} |
| 5 | {"s":null} |
+----+----------------------------+
However, in the first row it is expected that 's' should be null and not
its members; in the second row the result should be 'NULL', i.e.
'outer_struct.inner_struct3' is null.
For reference see what is returned when querying 'outer_struct' instead
of 'outer_struct.inner_struct3':
+----+-------------------------------------------------------------------------------------------------------------------------------+
| 4 | {"str":"","inner_struct1":{"str":"somestr2","de":12345.12},"inner_struct2":{"i":1,"str":"string"},"inner_struct3":{"s":null}} |
| 5 | {"str":null,"inner_struct1":null,"inner_struct2":null,"inner_struct3":null} |
+----+-------------------------------------------------------------------------------------------------------------------------------+
The problem comes from the incorrect handling of the different depths of
the following trees:
- the ORC type hierarchy (schema)
- the tuple descriptor / slot descriptor hierarchy
as the ORC type hierarchy contains a node for every level in the schema
but the tuple/slot descriptor hierarchy omits the levels of structs that
are not in the select list (but an ancestor of theirs is), as these
structs are not materialised.
In the case of the example query, the two hierarchies are the following:
ORC:
root --> outer_struct -> inner_struct3 -> s --> i
| \-> s
\-> id
Tuple/slot descriptors:
main_tuple --> inner_struct3 -> s --> i
| \-> s
\-> id
We create 'OrcColumnReader's for each node in the ORC type tree. Each
OrcColumnReader is assigned an ORC type node and a slot descriptor. The
incorrect behaviour comes from the incorrect pairing of ORC type nodes
with slot descriptors.
The old behaviour is described below:
Starting from the root, going along a path in both trees (for example
the path leading to outer_struct.inner_struct3.s.i), for each step we
consume a level in both trees until no more nodes remain in the
tuple/slot desc tree, and then we pair the last element from that tree
with the remaining ORC type node(s).
In the example, we get the following pairs:
(root, main_tuple) -> (outer_struct, inner_struct3) ->
(inner_struct3, s) -> (s, i) -> (i, i)
When we run out of structs in the tuple/slot desc tree, we still create
OrcStructReaders (because the ORC type is still a struct, but the slot
descriptor now refers to an INT), but we mark them incorrectly as
non-materialised.
Also, the OrcStructReaders for non-materialised structs do not need to
check for null-ness as they are not present in the select list, only
their descendants, and the ORC batch object stores null information also
for the descendants of null values.
Let's look at the row with id 4 in the example:
Because of the bug, the non-materialising OrcStructReader appears at the
level of the (s, i) pair, so the 's' struct is not checked for
null-ness, although it is actually null. One level lower, for 'i' (and
the inner 's' string field), the ORC batch object tells us that the
values are null (because their parent is). Therefore the nulls appear
one level lower than they should.
The correct behaviour is that ORC type nodes are paired with slot
descriptors if either
- the ORC type node matches the slot descriptor (they refer to the same
node in the schema) or
- the slot descriptor is a descendant of the schema node that the ORC
type node refers to.
This patch fixes the incorrect pairing of ORC types and slot
descriptors, so we have the following pairs:
(root, main_tuple) -> (outer_struct, main_tuple) ->
(inner_struct3, inner_struct3) -> (s, s) -> (i, i)
In this case the OrcStructReader for the pair (outer_struct, main_tuple)
becomes non-materialising and the one for (s, s) will be materialising,
so the 's' struct will also be null-checked, recognising null-ness at
the correct level.
This commit also fixes some comments in be/src/exec/orc-column-readers.h
and be/src/exec/hdfs-orc-scanner.h mentioning the field
HdfsOrcScanner::col_id_path_map_, which has been removed by
"IMPALA-10485: part(1): make ORC column reader creation independent of
schema resolution".
Testing:
- added tests to
testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test
that query various levels of the struct 'outer_struct' to check that
NULLs are at the correct level.
Change-Id: Iff5034e7bdf39c036aecc491fbd324e29150f040
Reviewed-on: http://gerrit.cloudera.org:8080/18403
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/exec/hdfs-orc-scanner.h | 6 ++---
be/src/exec/orc-column-readers.cc | 14 ++++++++---
be/src/exec/orc-column-readers.h | 2 +-
.../QueryTest/nested-struct-in-select-list.test | 29 +++++++++++++++++++++-
4 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/be/src/exec/hdfs-orc-scanner.h b/be/src/exec/hdfs-orc-scanner.h
index bdd40a933..477f0b7b5 100644
--- a/be/src/exec/hdfs-orc-scanner.h
+++ b/be/src/exec/hdfs-orc-scanner.h
@@ -46,13 +46,11 @@ class OrcComplexColumnReader;
/// * 'ProcessFileTail' to create orc::Reader with OrcMemPool and ScanRangeInputStream
/// * Resolve TupleDescriptors to get a list of mapped orc::Types (a.k.a column/node).
/// Init 'row_reader_options_' with these selected type ids.
-/// * Build a map 'col_id_path_map_' from each orc::Type id to a SchemaPath. Will be
-/// used in creating OrcColumnReaders.
/// * Create temporary orc::RowReader with 'row_reader_options_' to get the selected
/// subset of the schema (a tree of the selected orc::Types, i.e. the local variable
/// 'root_type' in 'HdfsOrcScanner::Open').
-/// * Create OrcColumnReaders recursively with 'root_type', 'col_id_path_map_' and
-/// TupleDescriptors (HdfsOrcScanner::Open)
+/// * Create OrcColumnReaders recursively with 'root_type' and TupleDescriptors
+/// (HdfsOrcScanner::Open)
/// * At the begining of processing a Stripe, we update 'row_reader_options_' to have
/// the range of the Stripe boundaries. Then create a orc::RowReader for this Stripe
/// (HdfsOrcScanner::NextStripe)
diff --git a/be/src/exec/orc-column-readers.cc b/be/src/exec/orc-column-readers.cc
index 83b6e3d47..644ac325f 100644
--- a/be/src/exec/orc-column-readers.cc
+++ b/be/src/exec/orc-column-readers.cc
@@ -48,12 +48,20 @@ OrcColumnReader* OrcColumnReader::Create(const orc::Type* node,
DCHECK(slot_desc != nullptr);
OrcColumnReader* reader = nullptr;
if (node->getKind() == orc::TypeKind::STRUCT) {
- if (slot_desc->type().IsStructType() &&
- slot_desc->children_tuple_descriptor() != nullptr) {
- // This is the case where we should materialize the struct and its children.
+ DCHECK_EQ(scanner->slot_to_col_id_.count(slot_desc), 1);
+ const bool node_matches_slot_desc =
+ scanner->slot_to_col_id_[slot_desc] == node->getColumnId();
+ if (node_matches_slot_desc && slot_desc->type().IsStructType()) {
+ // 'node' corresponds to 'slot_desc' and 'slot_desc' refers to a struct: this is the
+ // case where we should materialize the struct and its children.
+ // Note: 'node' could also correspond to 'slot_desc' if 'slot_desc' refers to an
+ // array.
+ DCHECK(slot_desc->children_tuple_descriptor() != nullptr);
reader = new OrcStructReader(node, slot_desc,
slot_desc->children_tuple_descriptor(), scanner);
} else {
+ // A descendant of 'node' corresponds to 'slot_desc'. The struct referenced by
+ // 'node' will not be materialised.
reader = new OrcStructReader(node, slot_desc, scanner);
}
} else if (node->getKind() == orc::TypeKind::LIST) {
diff --git a/be/src/exec/orc-column-readers.h b/be/src/exec/orc-column-readers.h
index 17195c99d..1fa79e592 100644
--- a/be/src/exec/orc-column-readers.h
+++ b/be/src/exec/orc-column-readers.h
@@ -69,7 +69,7 @@ class OrcColumnReader {
public:
/// Create a column reader for the given 'slot_desc' based on the ORC 'node'. We say
/// the 'slot_desc' and ORC 'node' match iff
- /// scanner->col_id_path_map_[node->getColumnId()] == slot_desc->col_path
+ /// scanner->slot_to_col_id_[slot_desc] == node->getColumnId()
/// Caller should guarantee that 'slot_desc' matches to ORC 'node' or one of its
/// descendants. If 'node' is a primitive type, 'slot_desc' should match it since
/// primitive types don't have descendants.
diff --git a/testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test b/testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test
index d010cdbef..e0b93a324 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test
@@ -130,6 +130,7 @@ select sub.id, sub.outer_struct from sub order by sub.id desc;
1,'{"str":"somestr1","inner_struct1":{"str":"somestr2","de":12345.12},"inner_struct2":{"i":333222111,"str":"somestr3"},"inner_struct3":{"s":{"i":112288,"s":null}}}'
---- TYPES
INT,STRING
+====
---- QUERY
# Checks that "SELECT nested_struct.* ..." omits the nested structs from the output.
select id, outer_struct.* from functional_orc_def.complextypes_nested_structs;
@@ -143,6 +144,32 @@ select id, outer_struct.* from functional_orc_def.complextypes_nested_structs;
INT,STRING
====
---- QUERY
+# IMPALA-10839: Display nulls at the correct level.
+select id, outer_struct.inner_struct3 from
+functional_orc_def.complextypes_nested_structs;
+---- RESULTS
+1,'{"s":{"i":112288,"s":null}}'
+2,'{"s":{"i":321,"s":"dfgs"}}'
+3,'NULL'
+4,'{"s":null}'
+5,'NULL'
+---- TYPES
+INT,STRING
+====
+---- QUERY
+# IMPALA-10839: Display nulls at the correct level.
+select id, outer_struct.inner_struct3.s from
+functional_orc_def.complextypes_nested_structs;
+---- RESULTS
+1,'{"i":112288,"s":null}'
+2,'{"i":321,"s":"dfgs"}'
+3,'NULL'
+4,'NULL'
+5,'NULL'
+---- TYPES
+INT,STRING
+====
+---- QUERY
# Subquery that returns a complex type is not supported.
# IMPALA-9500
select outer_struct
@@ -152,4 +179,4 @@ where outer_struct in
---- CATCH
AnalysisException: A subquery can't return complex types. (SELECT outer_struct FROM functional_orc_def.complex
types_nested_structs)
-====
\ No newline at end of file
+====