[jira] [Assigned] (DRILL-7434) TopNBatch constructs Union vector incorrectly

["Paul Rogers (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/DRILL-7434?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Paul Rogers reassigned DRILL-7434:
----------------------------------

    Assignee: Paul Rogers

> TopNBatch constructs Union vector incorrectly
> ---------------------------------------------
>
>                 Key: DRILL-7434
>                 URL: https://issues.apache.org/jira/browse/DRILL-7434
>             Project: Apache Drill
>          Issue Type: Bug
>            Reporter: Paul Rogers
>            Assignee: Paul Rogers
>            Priority: Major
>
> The Union type is an "experimental" type that has never been completed. Yet, we use it as if it works.
> Consider the test {{TestTopNSchemaChanges.testMissingColumn()}}. Run this with the new batch validator enabled. This test creates a union vector. Here is how the schema looks:
> {noformat}
> (UNION:OPTIONAL), subtypes=([FLOAT8, INT]),
>   children=([`internal` (MAP:REQUIRED), children=([`types` (UINT1:REQUIRED)])])
> {noformat}
> This is very hard to follow because the Union vector structure is complex (and has many issues.) Let's work though it.
> We are looking at the {{MaterializedField}} for the union vector. It tells us that this Union has two types: {{FLOAT8}} and {{INT}}. All good.
> The Union has a vector per type, stored in an "internal map".' That map shows up as child, it is there on the {{children}} list as {{internal}}. However, the metadata claims that only one vector exists in that map: the {{types}} vector (the one that tells us what type to use for each row.)  The vectors for {{FLOAT8}} and {{INT}} are missing.
> If, however, we use our debugger and inspect the actual contents of the {{internal}} map, we get the following:
> {noformat}
> [`internal` (MAP:REQUIRED), children=([`types` (UINT1:REQUIRED)], [`float8` (FLOAT8:OPTIONAL)], [`int` (INT:OPTIONAL)])]
> {noformat}
> That is, the internal map has the correct schema, but the Union vector itself has the wrong (incomplete) schema.
> This is an inherent design flaw with Union vector: it requires two copies of the schema to be in sync. Further {{MaterializedField}} was designed to be immutable, but the map and Union types require mutation. If the Union simply points to the actual Map vector {{MaterializedField}}, it will drift out of date since the map vector creates a new schema each time we add fields; the Union vector ends up pointing to the old one.
> This is not a simple bug to fix, but the result of the bug is that the vectors end up corrupted, as detected by the Batch Validator. In fact, the bug itself is subtle.
> The TopNBatch does pass vector validation. However, because of the incorrect metadata, the downstream {{RemovingRecordBatch}} creates the derived Union vector incorrectly: it fails to set the value count for the {{INT}} type.
> {noformat}
> Found one or more vector errors from RemovingRecordBatch
> kl-type-INT - NullableIntVector: Row count = 3, but value count = 0
> {noformat}
> Where {{kl-type-INT}} is an ad-hoc way of saying we are checking the {{INT}} type vector for a Union named {{kl}}.
> The schema of Union out of the {{RemovingRecordBatch}} has been truncated. The Union itself:
> {noformat}
> [`kl` (UNION:OPTIONAL), subtypes=([FLOAT8, INT]),
>   children=([`internal` (MAP:REQUIRED), children=([`types` (UINT1:REQUIRED)])])]
> {noformat}
> The internal map:
> {noformat}
> [`internal` (MAP:REQUIRED), children=([`types` (UINT1:REQUIRED)], [`int` (INT:OPTIONAL)])]
> {noformat}
> Notice that the {{FLOAT8}} vector has disappeared: the Union vector metadata claims we have such a vector, but the internal map does not actually contain the vector.
> The root cause is that the vector checker (indeed, any client) will call {{UnionVector.getMember(type)}} to get a vector for a type. This method includes a switch statement to call, say, {{getIntVector()}}. That method, in turn, creates the vector if does not exist.
> But, since we are reading, we have an existing data batch. When we create a new vector, we create it as zero size. Thus, we think we have n records (three in this case), but we actually have zero. This kinda-sorta works because the type vector won't ever contain an entry for the "runt" vector, so we won't actually access data. But, this is an inconsistent structure. It breaks if we peer inside, as we are doing in the batch validator.
> If we check for this case, we now get:
> {noformat}
> Found one or more vector errors from RemovingRecordBatch
> kl - UnionVector: Union vector includes type INT, but the internal map has no matching member
> {noformat}
> This is why Union is such a mess: is this a bug or just a very fragile design? I claim bug.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)