----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20491/#review41257 -----------------------------------------------------------
It's awesome work! There are no significant comments, and I leave some trivial comments. tajo-common/src/main/java/org/apache/tajo/datum/IntervalDatum.java <https://reviews.apache.org/r/20491/#comment74695> Could you remove the commented-out lines? tajo-common/src/main/java/org/apache/tajo/datum/IntervalDatum.java <https://reviews.apache.org/r/20491/#comment74705> Returning NULL may break the following processing. I suggest that the exception handling code just returns an empty string "" and records its error to some log. tajo-core/src/main/java/org/apache/tajo/engine/planner/ExprAnnotator.java <https://reviews.apache.org/r/20491/#comment74702> Please remove the commented-out lines. tajo-storage/src/main/java/org/apache/tajo/storage/LazyTuple.java <https://reviews.apache.org/r/20491/#comment74700> It should be removed. Returning NullDatum is a correct error-handling way. tajo-storage/src/main/java/org/apache/tajo/storage/RowStoreUtil.java <https://reviews.apache.org/r/20491/#comment74701> The byte arrays that RowStoreUtil is only used internally for intermediate data. We don't need to store it as a human-readable representation. We can just use a long value. - Hyunsik Choi On April 23, 2014, 7:37 p.m., hyoungjun kim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20491/ > ----------------------------------------------------------- > > (Updated April 23, 2014, 7:37 p.m.) > > > Review request for Tajo. > > > Bugs: TAJO-761 > https://issues.apache.org/jira/browse/TAJO-761 > > > Repository: tajo > > > Description > ------- > > In order to provide the following features, INTERVAL Type is required. > {noformat} > babokim# select timestamp '2001-09-28 01:00' + interval '23 hours'; > ?column? > --------------------- > 2001-09-29 00:00:00 > (1 row) > > babokim=# select date '2001-10-01' - date '2001-09-28'; > ?column? > ---------- > 3 > (1 row) > {noformat} > > > Diffs > ----- > > tajo-algebra/src/main/java/org/apache/tajo/algebra/IntervalLiteral.java > PRE-CREATION > tajo-algebra/src/main/java/org/apache/tajo/algebra/OpType.java a62ccfd > tajo-common/src/main/java/org/apache/tajo/datum/DateDatum.java c327931 > tajo-common/src/main/java/org/apache/tajo/datum/DatumFactory.java 2ddea9e > tajo-common/src/main/java/org/apache/tajo/datum/Float4Datum.java 0c65dfc > tajo-common/src/main/java/org/apache/tajo/datum/Float8Datum.java 010cf29 > tajo-common/src/main/java/org/apache/tajo/datum/Int2Datum.java e24d7be > tajo-common/src/main/java/org/apache/tajo/datum/Int4Datum.java 9cd0c06 > tajo-common/src/main/java/org/apache/tajo/datum/Int8Datum.java 1104a3c > tajo-common/src/main/java/org/apache/tajo/datum/IntervalDatum.java > PRE-CREATION > tajo-common/src/main/java/org/apache/tajo/datum/TimeDatum.java 21ae881 > tajo-common/src/main/java/org/apache/tajo/datum/TimestampDatum.java 6411bec > tajo-common/src/main/java/org/apache/tajo/json/DatumAdapter.java f24d213 > tajo-common/src/main/java/org/apache/tajo/util/TimeStampUtil.java 830c4aa > tajo-common/src/test/java/org/apache/tajo/datum/TestIntervalDatum.java > PRE-CREATION > tajo-common/src/test/java/org/apache/tajo/datum/TestTimestampDatum.java > 246791b > tajo-core/src/main/antlr4/org/apache/tajo/engine/parser/SQLLexer.g4 7fa7973 > tajo-core/src/main/antlr4/org/apache/tajo/engine/parser/SQLParser.g4 > f6385eb > tajo-core/src/main/java/org/apache/tajo/engine/eval/BinaryEval.java d362927 > > tajo-core/src/main/java/org/apache/tajo/engine/function/datetime/ToDate.java > PRE-CREATION > tajo-core/src/main/java/org/apache/tajo/engine/parser/SQLAnalyzer.java > cb356f8 > tajo-core/src/main/java/org/apache/tajo/engine/planner/AlgebraVisitor.java > aa94801 > > tajo-core/src/main/java/org/apache/tajo/engine/planner/BaseAlgebraVisitor.java > b8f3311 > tajo-core/src/main/java/org/apache/tajo/engine/planner/ExprAnnotator.java > 1b57b98 > tajo-core/src/main/java/org/apache/tajo/engine/planner/ExprNormalizer.java > 9030629 > tajo-core/src/main/java/org/apache/tajo/engine/planner/ExprsVerifier.java > 551393c > tajo-core/src/test/java/org/apache/tajo/engine/eval/TestIntervalType.java > PRE-CREATION > > tajo-core/src/test/java/org/apache/tajo/engine/function/TestDateTimeFunctions.java > ac7a2b8 > tajo-storage/src/main/java/org/apache/tajo/storage/LazyTuple.java 27d2691 > tajo-storage/src/main/java/org/apache/tajo/storage/RowStoreUtil.java > b64bfe1 > > tajo-storage/src/main/java/org/apache/tajo/storage/TextSerializerDeserializer.java > de73a3a > > Diff: https://reviews.apache.org/r/20491/diff/ > > > Testing > ------- > > mvn clean install; > tested in local cluster > > > Thanks, > > hyoungjun kim > >
