> On Feb. 5, 2015, 2:05 a.m., Yi Pan (Data Infrastructure) wrote: > > Could you link this to SAMZA-484?
Sure. > On Feb. 5, 2015, 2:05 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-sql/src/main/java/org/apache/samza/sql/data/string/StringData.java, > > line 29 > > <https://reviews.apache.org/r/30634/diff/1/?file=848470#file848470line29> > > > > May be we can add the default implementation to the interface class? Yeah. I wanted to have a separate follow-up RB for the refactored code. Since we are still in the Java world, requiring compatibility with JDK6, we should refactor the interface into abstract class. We should also consider a hierarchy of classes for supporting the access methods to various data forms (primitives, structs, record, map etc) > On Feb. 5, 2015, 2:05 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-sql/src/main/java/org/apache/samza/sql/data/string/StringSchema.java, > > line 18 > > <https://reviews.apache.org/r/30634/diff/1/?file=848471#file848471line18> > > > > Same here? Same as above, although I am still trying to figure out how to refactor the Schema interface > On Feb. 5, 2015, 2:05 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-sql/src/main/java/org/apache/samza/sql/operators/partition/PartitionOp.java, > > line 86 > > <https://reviews.apache.org/r/30634/diff/1/?file=848475#file848475line86> > > > > We should replace this with proper tuple API methods, i.e. this line > > should be: > > collector.send(new > > OutgoingMessageEnvelope(PartitionOp.this.spec.getSystemStream(), > > tuple.getKey().value(), > > tuple.getMessage().getFieldData(PartitionOp.this.spec.getParKey()).value(), > > tuple.getMessage().value())); > > > > This will illustrate how the schema APIs are used. Ah.. Didn't notice that. > On Feb. 5, 2015, 2:05 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-sql/src/main/java/org/apache/samza/sql/task/StoreMessageCollector.java, > > line 1 > > <https://reviews.apache.org/r/30634/diff/1/?file=848485#file848485line1> > > > > Same as the RandomOperatorTask, I have changed the package name to > > org.apache.samza.task.sql. Ah.. I see. Multiple patches are confusing. But isn't org.apache.samza.sql.task more suitable than task.sql ? Can you explain the different sub-modules under samza-sql? > On Feb. 5, 2015, 2:05 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-sql/src/test/java/org/apache/samza/sql/task/RandomOperatorTask.java, > > line 1 > > <https://reviews.apache.org/r/30634/diff/1/?file=848489#file848489line1> > > > > This is a redundant file. I changed the package name from > > org.apache.samza.sql.task to org.apache.samza.task.sql since both tasks are > > extended from org.apache.samza.task.StreamTask. The same for the > > StreamSqlTask. Ok > On Feb. 5, 2015, 2:05 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-sql/src/main/java/org/apache/samza/sql/data/serializers/SqlStringSerdeFactory.java, > > line 11 > > <https://reviews.apache.org/r/30634/diff/1/?file=848469#file848469line11> > > > > Can you add some comments or illustrate how the "name" is used here? The samza container parses the config to find all the serdeFactories used by the job (SamzaContainer.java: Line 141). Anything with "serializers.registry." substring is parsed and used to generate a map of form "serdeType => serdeFactory instance". This is used by SerdeManager for lookups. For example, if a job defines "serializers.registry.string.class=org.apache.samza.serializers.StringSerdeFactory", the key is string and the value is an instance of StringSerdeFactory. - Navina ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30634/#review71121 ----------------------------------------------------------- On Feb. 4, 2015, 7:39 p.m., Navina Ramesh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30634/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2015, 7:39 p.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > Changes return type in the tuple interface to Data, instead of Object > > > Wrapping default serdes within sql specific serdes > > > Synced with Yi's latest patch - added README > > > Removing unused enum SerdeType > > > Diffs > ----- > > build.gradle 4f9fb44d6dbf927a3ba92ef3c3dc91903406bb4f > gradle/dependency-versions.gradle 7bbaa41831c842b8164734ed578c07dd3394397f > samza-sql/README PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/api/data/Data.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/api/data/EntityName.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/api/data/Relation.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/api/data/Schema.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/api/data/Tuple.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/api/operators/Operator.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/api/operators/RelationOperator.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/api/operators/SqlOperatorFactory.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/api/operators/TupleOperator.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/api/router/OperatorRouter.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/data/IncomingMessageTuple.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/data/avro/AvroData.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/data/avro/AvroSchema.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/data/serializers/SqlStringSerde.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/data/serializers/SqlStringSerdeFactory.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/data/string/StringData.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/data/string/StringSchema.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/factory/SimpleOperator.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/partition/PartitionOp.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/partition/PartitionSpec.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/operators/relation/Join.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/relation/JoinSpec.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/stream/InsertStream.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/stream/InsertStreamSpec.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/window/BoundedTimeWindow.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/window/WindowSpec.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/operators/window/WindowState.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/sql/router/SimpleRouter.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/sql/task/StoreMessageCollector.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/task/sql/OperatorMessageCollector.java > PRE-CREATION > samza-sql/src/main/java/org/apache/samza/task/sql/SqlMessageCollector.java > PRE-CREATION > > samza-sql/src/main/java/org/apache/samza/task/sql/StoreMessageCollector.java > PRE-CREATION > samza-sql/src/test/java/org/apache/samza/sql/task/RandomOperatorTask.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 > settings.gradle 3a01fd66359b8c79954ae8f34eeaf4b2e3fdc0b4 > > Diff: https://reviews.apache.org/r/30634/diff/ > > > Testing > ------- > > > Thanks, > > Navina Ramesh > >
