Hello Gabor Kaszab, Zoltan Borok-Nagy, Vuk Ercegovac,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10711
to look at the new patch set (#3).
Change subject: IMPALA-7141 (part 1): clean up handling of default/dummy
partition
......................................................................
IMPALA-7141 (part 1): clean up handling of default/dummy partition
Currently, HdfsTable inconsistently uses the term "default partition"
to refer to two different concepts:
1) For unpartitioned tables, a single partition with ID 1 and no partition
keys is created and added to the partition map.
2) All tables have an additional partition added with partition ID -1
which acts as a sort of prototype for partition creation: when new
partitions are created during an INSERT operation, the file format
and other related options are copied out of this special partition.
This partition is inconsistently referred to as either the "default
partition" or the "dummy partition".
The handling of this second case (the partition with id -1) was somewhat
messy:
- the partition shows up in the partitionMap_ member, but does not show
up in the partitionIds_ member.
- almost all of the call sites that iterate through the partitions of an
HdfsTable instance ended up skipping over the dummy partition.
- several call sites called getPartitions().size() but then had to
adjust the result by subtracting one in order to actually count the
number of partitions in a table.
- similarly, test assertions had to assert that tables with 24
partitions had an expected partition map size of 25.
In order to address the above, this patch makes the following changes:
- getPartitions() and getPartitionMap() no longer include the dummy
partition. This removes a bunch of special case checks to skip over
the dummy partition or to adjust partition counts based on it.
- to clarify the purpose of this partition, references to it are renamed
to "prototype partition" instead of "default partition".
- when converting the HdfsTable to/from Thrift, the prototype partition
is included in its own field in the struct, instead of being stuffed
into the same map with the true partitions of the table. This reflects
the fact that this partition is special (eg missing fields like
'location' which otherwise are required for real partitions)
This change should should be entirely internal with no functional
differences. As such, the only testing changes are some fixes for
assertions on the Thrift serialized structures and other internals.
Change-Id: I15e91b50eb7c2a5e0bac8c33d603d6cd8cbaca2e
---
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/runtime/descriptors.cc
M be/src/runtime/descriptors.h
M common/thrift/CatalogObjects.thrift
M common/thrift/ImpalaInternalService.thrift
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/testutil/BlockIdGenerator.java
18 files changed, 163 insertions(+), 205 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/10711/3
--
To view, visit http://gerrit.cloudera.org:8080/10711
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I15e91b50eb7c2a5e0bac8c33d603d6cd8cbaca2e
Gerrit-Change-Number: 10711
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Vuk Ercegovac <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
