----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30287/#review69927 -----------------------------------------------------------
The examples look clean and neat. I really liked it. One high-level overview question: how do we map the operator classes to this intermediate layer of OM? That would probably help to understand how to translate from this OM to the operators. samza-sql/src/main/java/org/apache/samza/sql/om/ArithmeticExpression.java <https://reviews.apache.org/r/30287/#comment114764> Don't quite get the use of the <code>operands</code> here. From the enum type, it seems that the ArithmeticExpression is binary operators with <code>lhs</code> and <code>rhs</code>. Not sure what's the intension to have the additional list of operands here? samza-sql/src/main/java/org/apache/samza/sql/om/FromExpression.java <https://reviews.apache.org/r/30287/#comment114766> This "FromExpression" is confusing to me. IMO, from clause should just take one expression as parameter which defines a data source. It seems to me that FROM t1 JOIN t2 ON <conditions> should be considered as FROM <JOIN expression>, in which, JOIN expression has two operands t1 and t2 and condition as a compound logic expression following ON. Maybe, FROM should not be an expression. It simply signifies a data source in the statement. samza-sql/src/main/java/org/apache/samza/sql/om/PartitionByExpression.java <https://reviews.apache.org/r/30287/#comment114770> Conceptually, I would prefer to differentiate "clause" from "expression". IMO, the hierarchy should be: statement => clauses => expressions => operands. samza-sql/src/main/java/org/apache/samza/sql/om/Stream.java <https://reviews.apache.org/r/30287/#comment114776> Following Chris' comment on remote stream, if we defines a canonical name fashion for all remote and local streams, that would be better. I.e. all streams that are in memory are named as "local:astream" and all remote streams are named as "kafka:bstream". samza-sql/src/main/java/org/apache/samza/sql/om/TimeVaryingRelation.java <https://reviews.apache.org/r/30287/#comment114777> I had a feeling that the expression classes may look better organized if we categorize the expressions based on the output entity that they represent. i.e. essentially, window expression and join expression are expressions that all generate a table-type data source entity. - Yi Pan (Data Infrastructure) On Jan. 26, 2015, 10:04 p.m., Milinda Pathirage wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30287/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2015, 10:04 p.m.) > > > Review request for samza, Chris Riccomini and Yi Pan (Data Infrastructure). > > > Repository: samza > > > Description > ------- > > WIP: Stream SQL Object Model > > Overview view of the design: > > There are three main types of objects in this model > > * Expression: Can be a reference to a column of a table or a field of a tuple > or a property of a nested data structure, a arithmetic expression, a logical > expression, a function or expressions that can be used in where clauses, > having clauses, field based window clauses > * DataSource: Data source can be a stream, a table or a time varying relation > results from applying a window operator to a stream. (**I'm not sure whether > DataSource is the correct naming. We need to discuss this.**) > * Statement: Stream SQL statements like SELECT, INSERT and UPDATE > > Then we have a simple factory (**StreamSQLFactory**) which creates different > types of expressions. And I have implemented a fluent style API for > **Select** to demonstrate how building a query using this OM will looks like. > Two examples queries can be found in **StreamSQLSamples**. > > This is the first draft. Please feel free to comment on this. > > > Diffs > ----- > > build.gradle 7a40ad4 > gradle/wrapper/gradle-wrapper.properties 78596c0 > samza-sql/src/main/java/org/apache/samza/sql/om/AbstractDataSource.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/AbstractExpression.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/AliasedExpression.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/ArithmeticExpression.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/CompareExpression.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/FieldReference.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/FromExpression.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/FunctionExpressions.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/Literal.java PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/LogicalExpression.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/OrderByExpression.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/OrderByField.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/PartitionByExpression.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/Select.java PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/Stream.java PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/StreamSQLFactory.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/Table.java PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/TimeBasedWindow.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/TimeVaryingRelation.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/TupleBasedWindow.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/WindowExpression.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/api/DataSource.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/api/Expression.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/om/api/Statement.java > PRE-CREATION > samza-sql/src/test/java/org/apache/samza/sql/om/StreamSQLSamples.java > PRE-CREATION > settings.gradle 3a01fd6 > > Diff: https://reviews.apache.org/r/30287/diff/ > > > Testing > ------- > > > Thanks, > > Milinda Pathirage > >
