sijie commented on a change in pull request #4022: Making use of builtin
schemas for primitive types
URL: https://github.com/apache/pulsar/pull/4022#discussion_r274250231
##########
File path:
pulsar-sql/presto-pulsar/src/main/java/org/apache/pulsar/sql/presto/PulsarRecordCursor.java
##########
@@ -161,6 +161,47 @@ private SchemaHandler getSchemaHandler(Schema schema,
SchemaType schemaType,
case AVRO:
schemaHandler = new AvroSchemaHandler(schema, columnHandles);
break;
+ case STRING:
Review comment:
There is a method in `AutoConsumeSchema#getSchema` (which can be moved to
Schema.java => Schema.getSchema(SchemaType type)` to retrieve a schema instance
using SchemaInfo.
So the logic here can be simplified with
```
schemaHandler = new PrimitiveSchemaHandler(Schema.getSchema(schemaType));
```
also I think JSON and Avro schema handler should be changed to use pulsar
schema for deserialization. because there are changes adding there to support
schema versioning. If we still have a separated schema handler in presto, we
can't benefit from the changes happening in pulsar schemas. although this
change should be done in a separate PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
