gszadovszky commented on code in PR #1296:
URL: https://github.com/apache/parquet-mr/pull/1296#discussion_r1525839145
##########
doc/dremel_paper/schema.png:
##########
Review Comment:
Same as above
##########
parquet-avro/src/test/java/org/apache/parquet/avro/TestAvroRecordConverter.java:
##########
Review Comment:
It seems you have rewritten the whole unit test. Was it necessary to do so?
I would feel more comfortable if the existing tests would pass as they were.
##########
parquet-avro/src/main/java/org/apache/parquet/avro/AvroRecordConverter.java:
##########
@@ -215,31 +263,7 @@ static SpecificData getModelForSchema(Schema schema) {
final String avroVersion = getRuntimeAvroVersion();
// Avro 1.7 and 1.8 don't include conversions in the MODEL$ field by
default
if (avroVersion != null && (avroVersion.startsWith("1.8.") ||
avroVersion.startsWith("1.7."))) {
- final Field conversionsField;
- try {
- conversionsField = clazz.getDeclaredField("conversions");
- } catch (NoSuchFieldException e) {
- // Avro classes without logical types (denoted by the "conversions"
field) can be returned as-is
- return model;
- }
-
- final Conversion<?>[] conversions;
- try {
- conversionsField.setAccessible(true);
- conversions = (Conversion<?>[]) conversionsField.get(null);
- } catch (IllegalAccessException e) {
- LOG.warn(String.format(
- "Field `conversions` in class %s was inaccessible. Parquet will
use default "
- + "SpecificData model for reading and writing.",
- clazz));
- return null;
Review Comment:
At this point we were returning a `null` as the model. The new code does not
seem to do so. Is it intentional and correct?
##########
doc/dremel_paper/dremel_example.png:
##########
Review Comment:
Something went odd with the formatting. It shouldn't touch binary files.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
