Mailing-List: contact dev-help@drill.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@drill.apache.org
Content-Type: text/plain; charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\))
Subject: Re: isDateCorrect field in ParquetTableMetadata
From: Paul Rogers <progers@maprtech.com>
In-Reply-To: <CADN0Fn_Rzaxt4Hyo7=XmF3ahUs4k7ixB+6TuKL-es3EVSjvBUA@mail.gmail.com>
Date: Fri, 28 Oct 2016 09:26:08 -0700
Content-Transfer-Encoding: quoted-printable
Message-Id: <1466CA7E-E4E5-4D22-A655-5B82A5D58EB7@maprtech.com>
References: <CAK55V=J13pBF7Xx3Peyjk9sQVzO=BCVZvjoWSYZaphMH_=bg+Q@mail.gmail.com> <CAK55V=+m=aBTb=52bM54v2EF66cWDn5_m49EF42zNk=vJ7+uKg@mail.gmail.com> <CAK55V=LKBmkW05Xvyq7dSJS_Ysm40XGA-9f_Dr2F+iuxNgqKmA@mail.gmail.com> <CADN0Fn_DoVqD+ysvPRC9Ct-E796q9Q0kw+ojGwePyitqMuCzTA@mail.gmail.com> <CAK55V=JwOzcSEBtxoe7nj9tQhqo=PVVm17YT003rJq5zz_DT=Q@mail.gmail.com> <CADN0Fn8Kn4Ff7ULtUQg0hfXAG-TX58gJEV9xfmJaUnpVp5edYA@mail.gmail.com> <CACX89XeETWAqCQuQFohLLZJqwf4AwAe1Aimy70ZVSCPvhMh-jg@mail.gmail.com> <95CF8C46-AF7F-4FFF-A5D2-9668C5330FD6@maprtech.com> <CADN0Fn_Rzaxt4Hyo7=XmF3ahUs4k7ixB+6TuKL-es3EVSjvBUA@mail.gmail.com>
To: "dev@drill.apache.org" <dev@drill.apache.org>
archived-at: Fri, 28 Oct 2016 16:26:17 -0000

Thanks Vitalii.

The Parquet Writer solution =E2=80=9Cjust works=E2=80=9D. As soon as =
someone upgrades the writer, files are labeled as having that new =
version. No fuzziness during a release as in 1.9.

It is fine to also include the Drill version. But, format decisions =
should be keyed off of the writer version.

By the way, do other tools happen to already do this? It would be rather =
surprising if they didn=E2=80=99t.

- Paul

> On Oct 28, 2016, at 8:30 AM, Vitalii Diravka =
<vitalii.diravka@gmail.com> wrote:
>=20
> I agree that it would be good if the approach of parquet date =
correctness
> detection will be upgraded. So I created the jira for it DRILL-4980
> <https://issues.apache.org/jira/browse/DRILL-4980>.
>=20
> But now we have two ideas:
> 1. To add checking of the drill version additionally, so later we can
> delete isDateCorrect label from parquet metadata.
> 2. To add parquet writer version to the parquet metadata and check =
this
> value instead of isDateCorrect and drillVersion.
>=20
> So which way, we should prefer now?
>=20
> Kind regards
> Vitalii
>=20
> 2016-10-27 23:54 GMT+00:00 Paul Rogers <progers@maprtech.com>:
>=20
>> FWIW: back on the magic flag issue=E2=80=A6
>>=20
>> I noted Vitali=E2=80=99s concern about =E2=80=9C1.9=E2=80=9D and =
=E2=80=9C1.9-SNAPSHOT=E2=80=9D being too course
>> grained for our needs.
>>=20
>> A typical solution is include the version of the Parquet writer in
>> addition to that of Drill. Each time we change something in the =
writer,
>> increment the version number. If we number changes, we can easily =
handle
>> two changes in the same Drill release, or differentiate between the =
=E2=80=9Cearly
>> 1.9=E2=80=9D files with old-style dates and =E2=80=9Clate 1.9=E2=80=9D =
files with correct dates.
>>=20
>> Since we have no version now, start it at some arbitrary point (2?).
>>=20
>> Now, if the Parquet file has a Drill Writer version in the header, =
and
>> that version is 2 or greater, the date is in the =E2=80=9Ccorrect=E2=80=
=9D format. Anything
>> written by Drill before writer version 2, the date is wrong. The =
=E2=80=9Ccheck the
>> data to see if it is sane=E2=80=9D approach is needed only for files =
were we can=E2=80=99t
>> tell if an older Drill wrote it.
>>=20
>> Do other tools label the data? Does Hive say that it wrote the file? =
If
>> so, we don=E2=80=99t need to do the sanity check if we can tell the =
data comes from
>> Hive (or Impala, or anything other than old Drill.)
>>=20
>> - Paul
>>=20
>>> On Oct 27, 2016, at 4:03 PM, Zelaine Fong <zfong@maprtech.com> =
wrote:
>>>=20
>>> Vitalii -- are you still planning to open a ticket and pull request =
for
>> the
>>> fix you've noted below?
>>>=20
>>> -- Zelaine
>>>=20
>>> On Wed, Oct 26, 2016 at 8:28 AM, Vitalii Diravka <
>> vitalii.diravka@gmail.com>
>>> wrote:
>>>=20
>>>> @Paul Rogers
>>>> It may be the undefined case when the file is generated with
>> drill.version
>>>> =3D 1.9-SNAPSHOT.
>>>> It is more easy to determine corrupted date with this flag and =
there is
>> no
>>>> need to wait the end of release to merge these changes.
>>>>=20
>>>> @Jinfeng NI
>>>> It looks like you are right.
>>>> With consistent mode (isDateCorrect =3D true) all tests are passed. =
So I
>> am
>>>> going to open a jira ticket for it with next changes
>>>> =
https://github.com/vdiravka/drill/commit/ff8d5c7d601915f760d1b0e9618730
>>>> 3410cac5d3
>>>> Thanks.
>>>>=20
>>>> Kind regards
>>>> Vitalii
>>>>=20
>>>> 2016-10-25 18:36 GMT+00:00 Jinfeng Ni <jni@apache.org>:
>>>>=20
>>>>> I'm not sure if I fully understand your answers. The bottom line =
is
>>>>> quite simple: given a set of parquet files, the ParquetTableMeta
>>>>> instance constructed in Drill should have identical value for
>>>>> "isDateCorrect", whether it comes from parquet footer, or parquet
>>>>> metadata cache, or whether there is partition pruning or not. =
However,
>>>>> the code shows that this flag is not in consistent mode across
>>>>> different cases.
>>>>>=20
>>>>>=20
>>>>>=20
>>>>> On Tue, Oct 25, 2016 at 11:24 AM, Vitalii Diravka
>>>>> <vitalii.diravka@gmail.com> wrote:
>>>>>> Hi Jinfeng,
>>>>>>=20
>>>>>> 1.If the parquet files are generated with Drill after Drill-4203 =
these
>>>>>> files have "isDateCorrect =3D true" property.
>>>>>> Drill serializes this property from metadata now. When we set =
this
>>>>> property
>>>>>> in the first constructor we will hide the value from metadata.
>>>>>> IsDateCorrect will be false only if this value equals to the =
false (no
>>>>> case
>>>>>> for it now) or absent in parquet metadata footer.
>>>>>>=20
>>>>>>=20
>>>>>> 2. I'm not sure the reason to change isDateCorrect metadata =
property
>>>> when
>>>>>> the user disable dates correction.
>>>>>> If you have some use case it would be great if you provide it.
>>>>>>=20
>>>>>> 3. Maybe you are right regarding to when Parquet metadata is =
cloned.
>>>>>> Here I added the property in the same manner as Jason's new =
property
>>>>>> "drillVersion. So need it a separate unit test?
>>>>>>=20
>>>>>>=20
>>>>>> Kind regards
>>>>>> Vitalii
>>>>>>=20
>>>>>> 2016-10-25 16:23 GMT+00:00 Jinfeng Ni <jni@apache.org>:
>>>>>>=20
>>>>>>> Forgot to copy the link to the code.
>>>>>>>=20
>>>>>>> [1] https://github.com/apache/drill/blob/master/exec/java-
>>>>>>> exec/src/main/java/org/apache/drill/exec/store/parquet/
>>>>>>> Metadata.java#L950-L955
>>>>>>>=20
>>>>>>> On Tue, Oct 25, 2016 at 9:16 AM, Jinfeng Ni <jni@apache.org> =
wrote:
>>>>>>>> @Jason, @Vitalli,
>>>>>>>>=20
>>>>>>>> Any thoughts on this question, since both you worked on fix of
>>>>>>> DRILL-4203?
>>>>>>>>=20
>>>>>>>> Looking through the code, there is a third case [1], where this =
flag
>>>>>>>> is set to false when Parquet metadata is cloned (after =
partition
>>>>>>>> pruning, etc).  That means, for the 2nd case where the flag is =
set
>>>> to
>>>>>>>> true, if there is pruning happening, the new parquet metadata =
will
>>>> see
>>>>>>>> the flag is flipped to false. This does not make sense to me.
>>>>>>>>=20
>>>>>>>>=20
>>>>>>>>=20
>>>>>>>> On Mon, Oct 24, 2016 at 3:10 PM, Jinfeng Ni <jni@apache.org> =
wrote:
>>>>>>>>> Hello All,
>>>>>>>>>=20
>>>>>>>>> DRILL-4203 addressed the date field issue.  In the fix, it
>>>> introduced
>>>>>>>>> a new field in ParquetTableMetadata_v2 : isDateCorrect.  I =
have
>>>> some
>>>>>>>>> difficulty in understanding the meaning of this field.
>>>>>>>>>=20
>>>>>>>>> According to [1], this field is set to false, when Drill gets
>>>> parquet
>>>>>>>>> metadata from parquet footer.  This field is  set to true in =
code
>>>>> flow
>>>>>>>>> of [2] and [3], when Drill gets parquet metadata from meta =
data
>>>>> cache.
>>>>>>>>>=20
>>>>>>>>> Questions I have:
>>>>>>>>> 1.  If the parquet files are generated with Drill after =
DRILL-4203,
>>>>>>>>> Drill still thinks date field is NOT correct (isDateCorrect =3D
>>>> false)?
>>>>>>>>> 2.  Why does this filed have nothing to do with =
"autoCorrection"
>>>> flag
>>>>>>>>> [4]?  If someone turns off autoCorrection, will it have impact =
on
>>>>> this
>>>>>>>>> "isDateCorrect" flag ?
>>>>>>>>>=20
>>>>>>>>> Thanks in advance for any input,
>>>>>>>>>=20
>>>>>>>>> Jinfeng
>>>>>>>>>=20
>>>>>>>>>=20
>>>>>>>>> [1] https://github.com/apache/drill/blob/master/exec/java-
>>>>>>> exec/src/main/java/org/apache/drill/exec/store/parquet/
>>>>> Metadata.java#L932
>>>>>>>>> [2] https://github.com/apache/drill/blob/master/exec/java-
>>>>>>> exec/src/main/java/org/apache/drill/exec/store/parquet/
>>>>> Metadata.java#L936
>>>>>>>>> [3] https://github.com/apache/drill/blob/master/exec/java-
>>>>>>> exec/src/main/java/org/apache/drill/exec/store/parquet/
>>>>> Metadata.java#L187
>>>>>>>>> [4] https://github.com/apache/drill/blob/master/exec/java-
>>>>>>> exec/src/main/java/org/apache/drill/exec/store/parquet/
>>>>>>> Metadata.java#L354-L355
>>>>>>>=20
>>>>>=20
>>>>=20
>>=20
>>=20