You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/09/11 17:45:11 UTC
[GitHub] [incubator-pinot] jackjlli opened a new pull request #6005: Fix extract method in AvroRecordExtractor class
jackjlli opened a new pull request #6005:
URL: https://github.com/apache/incubator-pinot/pull/6005
## Description
This PR fixes the extract method in AvroRecordExtractor class.
When `_extractAll` is true, the generic record will be first converted to a json String and then parse to a json map, whereas json object has its precision limitation:
https://developers.google.com/discovery/v1/type-format
E.g. the column1 was originally of long type. But when it was converted to json string and then parse to a json map, the type would be changed to int. Thus, the actual value will be incorrect from the json map comparing to the actual value from generic record.
This PR fixes it by fetching the actual value directly from the original generic record.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on pull request #6005: Fix extract method in AvroRecordExtractor class
Posted by GitBox <gi...@apache.org>.
jackjlli commented on pull request #6005:
URL: https://github.com/apache/incubator-pinot/pull/6005#issuecomment-691313748
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on pull request #6005: Fix extract method in AvroRecordExtractor class
Posted by GitBox <gi...@apache.org>.
jackjlli commented on pull request #6005:
URL: https://github.com/apache/incubator-pinot/pull/6005#issuecomment-691313748
> Could you add a test such that it fails with the original code, and passes with the old code?
Test added.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #6005: Fix extract method in AvroRecordExtractor class
Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #6005:
URL: https://github.com/apache/incubator-pinot/pull/6005#discussion_r487224829
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Done.
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Done.
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Done.
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Done.
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Done.
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Done.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6005: Fix extract method in AvroRecordExtractor class
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6005:
URL: https://github.com/apache/incubator-pinot/pull/6005#discussion_r487213705
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Suggest using the non-functional way for performance concern
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on pull request #6005: Fix extract method in AvroRecordExtractor class
Posted by GitBox <gi...@apache.org>.
jackjlli commented on pull request #6005:
URL: https://github.com/apache/incubator-pinot/pull/6005#issuecomment-691313748
> Could you add a test such that it fails with the original code, and passes with the old code?
Test added.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #6005: Fix extract method in AvroRecordExtractor class
Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #6005:
URL: https://github.com/apache/incubator-pinot/pull/6005#discussion_r487224829
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Done.
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Done.
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Done.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6005: Fix extract method in AvroRecordExtractor class
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6005:
URL: https://github.com/apache/incubator-pinot/pull/6005#discussion_r487213705
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Suggest using the non-functional way for performance concern
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Suggest using the non-functional way for performance concern
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Suggest using the non-functional way for performance concern
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Suggest using the non-functional way for performance concern
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Suggest using the non-functional way for performance concern
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Suggest using the non-functional way for performance concern
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Suggest using the non-functional way for performance concern
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Suggest using the non-functional way for performance concern
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6005: Fix extract method in AvroRecordExtractor class
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6005:
URL: https://github.com/apache/incubator-pinot/pull/6005#discussion_r487213705
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Suggest using the non-functional way for performance concern
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #6005: Fix extract method in AvroRecordExtractor class
Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #6005:
URL: https://github.com/apache/incubator-pinot/pull/6005#discussion_r487224829
##########
File path: pinot-plugins/pinot-input-format/pinot-avro-base/src/main/java/org/apache/pinot/plugin/inputformat/avro/AvroRecordExtractor.java
##########
@@ -46,14 +46,13 @@ public void init(Set<String> fields, @Nullable RecordExtractorConfig recordExtra
@Override
public GenericRow extract(GenericRecord from, GenericRow to) {
if (_extractAll) {
- Map<String, Object> jsonMap = JsonUtils.genericRecordToJson(from);
- jsonMap.forEach((fieldName, value) -> to.putValue(fieldName, AvroUtils.convert(value)));
+ List<Schema.Field> fields = from.getSchema().getFields();
Review comment:
Done.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli merged pull request #6005: Fix extract method in AvroRecordExtractor class
Posted by GitBox <gi...@apache.org>.
jackjlli merged pull request #6005:
URL: https://github.com/apache/incubator-pinot/pull/6005
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on pull request #6005: Fix extract method in AvroRecordExtractor class
Posted by GitBox <gi...@apache.org>.
jackjlli commented on pull request #6005:
URL: https://github.com/apache/incubator-pinot/pull/6005#issuecomment-691313748
> Could you add a test such that it fails with the original code, and passes with the old code?
Test added.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org