----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33142/#review80897 -----------------------------------------------------------
Ship it! Looks very sound, for the stage we are at. Nice work. I had a couple of comments on Schema. I think you should add DATE, TIMESTAMP and INTERVAL types, because they are very important to streaming. Also, you should consider allowing people to declare that fields are not null. It will be difficult to add efficient code-generation later if you don't know whether nulls are possible. - Julian Hyde On April 13, 2015, 9:04 p.m., Yi Pan (Data Infrastructure) wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33142/ > ----------------------------------------------------------- > > (Updated April 13, 2015, 9:04 p.m.) > > > Review request for samza and Milinda Pathirage. > > > Bugs: SAMZA-561 > https://issues.apache.org/jira/browse/SAMZA-561 > > > Repository: samza > > > Description > ------- > > [SAMZA-561] Review in progress > > Post Milinda's patch for SAMZA-561 to ease the comment and discussion. > > > Diffs > ----- > > build.gradle 97de3a28f6379e3862eec845da87587b1d4f742e > gradle/dependency-versions.gradle ee6dfc411b7ab90b187df79f109884127953862e > samza-sql/src/main/java/org/apache/samza/sql/Utils.java PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/api/operators/spec/OperatorSpec.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/data/IntermediateMessageTuple.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/data/serializers/SqlAvroSerdeFactory.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/expressions/Expression.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/expressions/RexToJavaCompiler.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/expressions/RexToJavaUtils.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/metadata/AvroSchemaConverter.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/metadata/RelDataTypeToAvroSchemaConverter.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/metadata/Stream.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/factory/SimpleOperatorFactoryImpl.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/factory/SimpleOperatorSpec.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/factory/TypeAwareOperatorSpec.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/insert/InsertToStreamOp.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/insert/InsertToStreamSpec.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/project/ProjectOp.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/project/ProjectSpec.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/scan/ProjectableFilterableStreamScanOp.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/scan/ProjectableFilterableStreamScanSpec.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/scan/StreamScanSpec.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/planner/ExecutionPlanner.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/planner/QueryPlanner.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/planner/rules/FilterableStreamScanRule.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/planner/rules/ProjectableStreamScanRule.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/planner/rules/RemoveIdentityProjectRule.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/rel/ProjectableFilterableStreamScan.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/rel/StreamScan.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/task/sql/StreamSqlTask.java > PRE-CREATION > > samza-sql/src/test/java/org/apache/samza/sql/data/serializers/SqlAvroSerdeTest.java > PRE-CREATION > samza-sql/src/test/java/org/apache/samza/sql/planner/QueryPlannerTest.java > PRE-CREATION > > samza-sql/src/test/java/org/apache/samza/sql/planner/SamzaStreamTableFactory.java > PRE-CREATION > > samza-sql/src/test/java/org/apache/samza/sql/planner/TestExecutionPlanner.java > PRE-CREATION > samza-sql/src/test/java/org/apache/samza/sql/planner/TestQueryPlanner.java > PRE-CREATION > > samza-sql/src/test/java/org/apache/samza/sql/planner/TestRexToJavaCompiler.java > PRE-CREATION > samza-sql/src/test/java/org/apache/samza/sql/test/Constants.java > PRE-CREATION > samza-sql/src/test/java/org/apache/samza/sql/test/Utils.java PRE-CREATION > > samza-sql/src/test/java/org/apache/samza/sql/test/metadata/TestAvroSchemaConverter.java > PRE-CREATION > samza-sql/src/test/java/org/apache/samza/task/sql/RandomOperatorTask.java > PRE-CREATION > samza-sql/src/test/java/org/apache/samza/task/sql/StreamSqlTask.java > PRE-CREATION > samza-sql/src/test/resources/orders.avsc PRE-CREATION > samza-sql/src/test/resources/orders.json PRE-CREATION > samza-test/src/main/config/sql-filter.properties PRE-CREATION > > samza-test/src/main/java/org/apache/samza/test/integration/sql/OrdersStreamFactory.java > PRE-CREATION > samza-test/src/main/java/org/apache/samza/test/integration/sql/SqlTask.java > PRE-CREATION > samza-test/src/main/python/integration_tests.py > df64e239a2e467c8e4429dbeb7039f1aa9965ecc > samza-test/src/main/python/requirements.txt > 2ae95908248516b5b26e671f24fa680f7b801675 > samza-test/src/main/python/samza_job_yarn_deployer.py > 38635ca5899c43fb61d6b4042e8543f0508fd41b > samza-test/src/main/python/tests/sql_tests.py PRE-CREATION > samza-test/src/main/resources/orders.avsc PRE-CREATION > samza-test/src/main/resources/orders.json PRE-CREATION > > Diff: https://reviews.apache.org/r/33142/diff/ > > > Testing > ------- > > > Thanks, > > Yi Pan (Data Infrastructure) > >
