impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-2716: Hive/Impala incompatibility for timestamp data in Parquet
Date Tue, 04 Apr 2017 05:31:20 GMT
Alex Behm has posted comments on this change.

Change subject: IMPALA-2716: Hive/Impala incompatibility for timestamp data in Parquet

Patch Set 5:


I'm pretty happy with the code! Moving on to the tests.
File be/src/exec/hdfs-scan-node-base.h:

Line 305:   // parquet-mr.
Comment that the FE guarantees that this is a valid timezone.
File be/src/exec/

Line 246:       DCHECK((is_timestamp_dependent_timezone_ && timezone_ == NULL) ||
simplify condition:

is_timestamp_dependent_timezone_ == (timezone_ == NULL)

Line 549:   /// Used to cache the timezone object corresponding to ""
the "" table property

Line 550:   /// table property to avoid repeated calls to 'TimezoneDatabase::FindTimezone()'.
no need to single-quote here
File be/src/exprs/

Line 190:   // See if they specified a zone id
Who is "they"? Suggest rephrasing

Line 202:   return time_zone_ptr();
return NULL?
File common/thrift/BackendGflags.thrift:

Line 58:   // If true, all newly created Parquet tables will have the
... all newly created HDFS tables regardless of format ...
File common/thrift/PlanNodes.thrift:

Line 218:   9: optional string parquet_mr_write_zone;
remove trailing semicolon for consistency
File fe/src/main/java/org/apache/impala/analysis/

Line 112:       throw new AnalysisException("Invalid Time Zone: " + timezone);
Mention that the timezone is in the table property ''. Otherwise,
the user might have no clue where this timezone comes from.
File fe/src/main/java/org/apache/impala/analysis/

Line 186:             "Only HDFS tables can use '%s' table property.",
the '%s' table property

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f24525ef45a2814f476bdee76655b30081079d6
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Attila Jeges <>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Taras Bobrovytsky <>
Gerrit-Reviewer: Zoltan Ivanfi <>
Gerrit-HasComments: Yes

View raw message