You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by GitBox <gi...@apache.org> on 2020/11/03 13:33:04 UTC
[GitHub] [avro] wernerdaehn opened a new pull request #973: AVRO-2952: AvroDatatype
wernerdaehn opened a new pull request #973:
URL: https://github.com/apache/avro/pull/973
Make sure you have checked _all_ steps below.
### Jira
- [Avro Jira](https://issues.apache.org/jira/browse/AVRO-2952)
### Tests
- org.apache.avro.generic.TestDatatype
### Commits
- [ ] My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](https://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation
- All the public functions and the classes in the PR contain Javadoc that explain what it does
----------------------------------------------------------------
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
[GitHub] [avro] wernerdaehn commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
wernerdaehn commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-773465174
Thanks @RyanSkraba, appreciate it. The important part to keep in mind is that using this interface is optional. It neither impacts existing Logical Data Types nor custom data types - I have these myself. And it does not change how you can use existing or custom conversions. It just has easier to use helper classes for the normal cases.
Regarding the amount of code, actually most is just a repeat. If you scanned one data type class you know all. The only parts that will be interesting are the changes I made in the LogicalType.java and Schema.java. The first requires to change a few properties to PUBLIC and both include - unfortunately - methods that deal with individual logical types.
Anyway, once you read the code you will find that it is all pretty straight forward.
----------------------------------------------------------------
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
[GitHub] [avro] wernerdaehn commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
wernerdaehn commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-771703330
@RyanSkraba
Ready for review!
Just for a side note, I have some more code that I could contribute to Avro. But before doing that, I need to know if this one got accepted and what I can do better in future.
----------------------------------------------------------------
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
[GitHub] [avro] wernerdaehn commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
wernerdaehn commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-766171549
@unchuckable
Hi Martin, I made the agreed changes and more of what you have requested initially. For example the hashCode(), the toString(StringBuffer) etc.
Will check in the code as soon as the main build is on PASS again.
----------------------------------------------------------------
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
[GitHub] [avro] unchuckable commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
unchuckable commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-744046565
While I am no maintainer, I'll still offer my two cents from a short browse over what seems like a very interesting PR.
This PR features an interesting approach to extending the handling of logical types. I like the general concept, but I personally feel that the code as presented is not as mature (or I don't understand it enough) as I'd like to see code that's merged into my favourite serialization framework ;)
Points that should be addressed in my eyes:
* the methods `public String toString()` and `public String toString(StringBuilder, Value)` imply they do something similar, but actually don't
* the methods `public int hashCode()` are not implemented consistently, and where they are, they all return 1 which leads to bad hashmapping in cases where that might ever be used
* `public boolean equals()` feels odd where it addresses Arrays and Maps
* instances of override functions that only call their super version are redundant
* it is unclear to me why some effectively static final fields are only set as static (leads to short lived wtfs when reading)
* `getRecommendedSchema` returns null in some cases (arrays, maps, records), but neither interface or javadoc specify that
I wonder if it would be possible to reduce the PR size a lot by using a parameterized base class for the most common datatypes and only override code that actually changes between the different classes. (functions like `getBackingType`, `getRecommendedSchema`, `getAvroType` and `getConvertedType` are basically the same in all cases)
Again, as I said above, I like the general concept, and I am sure the presented PR can be "prettied up" quite a bit to be easier readable and understandable.
----------------------------------------------------------------
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
[GitHub] [avro] unchuckable commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
unchuckable commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-766172140
Hi, Werner,
that’s nice to hear. (Also good to hear about the main build not being on PASS, because that could explain issues with my own PR ;)). Mind you that I cannot accept your PR but was merely trying to offer constrictive remarks that might be made by others during the review. Lets hope one of the commited maintainers can find the time to review timely.
> On 23. Jan 2021, at 21:05, wernerdaehn <no...@github.com> wrote:
>
>
> @unchuckable <https://github.com/unchuckable>
> Hi Martin, I made the agreed changes and more of what you have requested initially. For example the hashCode(), the toString(StringBuffer) etc.
> Will check in the code as soon as the main build is on PASS again.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub <https://github.com/apache/avro/pull/973#issuecomment-766171549>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEDPCL4NPOPNAPEUKPS3HRDS3MT2BANCNFSM4TH4WI3A>.
>
----------------------------------------------------------------
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
[GitHub] [avro] wernerdaehn commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
wernerdaehn commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-773465174
Thanks @RyanSkraba, appreciate it. The important part to keep in mind is that using this interface is optional. It neither impacts existing Logical Data Types nor custom data types - I have these myself. And it does not change how you can use existing or custom conversions. It just has easier to use helper classes for the normal cases.
Regarding the amount of code, actually most is just a repeat. If you scanned one data type class you know all. The only parts that will be interesting are the changes I made in the LogicalType.java and Schema.java. The first requires to change a few properties to PUBLIC and both include - unfortunately - methods that deal with individual logical types.
Anyway, once you read the code you will find that it is all pretty straight forward.
----------------------------------------------------------------
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
[GitHub] [avro] wernerdaehn commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
wernerdaehn commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-771703330
@RyanSkraba
Ready for review!
Just for a side note, I have some more code that I could contribute to Avro. But before doing that, I need to know if this one got accepted and what I can do better in future.
----------------------------------------------------------------
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
[GitHub] [avro] emkornfield commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
emkornfield commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-789447944
I didn't see mention in the JIRA or here, but was this discussed on the mailing list? IIUC this is adding on custom extensions to logical types and additional metadata to the schema? If this is the case it seems like something that other implementations might care about if they will need to support later (i.e. it seems like updating the specification first with optional these optional extensions would be a good thing to do first so there is agreement on how to interpret them). Otherwise i would worry about the java code becoming the specification (which in my experience yields lower quality implementations elsewhere). If this doesn't affect serialized data or interpretation in any way please ignore my comment.
----------------------------------------------------------------
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
[GitHub] [avro] wernerdaehn commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
wernerdaehn commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-789128144
A quick response: Agree with all you said. We need to make some decisions but whatever you decide, I am fine with.
----------------------------------------------------------------
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
[GitHub] [avro] wernerdaehn commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
wernerdaehn commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-814102198
I have brought this pull request to the current version for easier integration. Especially that one dangling test get replaced by the master version removing my changes.
Don't no what else to do and close of giving up.
--
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
[GitHub] [avro] RyanSkraba commented on a change in pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
RyanSkraba commented on a change in pull request #973:
URL: https://github.com/apache/avro/pull/973#discussion_r585662437
##########
File path: lang/java/avro/src/main/java/org/apache/avro/logicaltypes/AvroByte.java
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.logicaltypes;
+
+import org.apache.avro.AvroTypeException;
+import org.apache.avro.LogicalType;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+
+/**
+ * Based on an INT but is supposed to hold data from -127 to +128 only. A single
Review comment:
A "Byte" logical type, if it were to exist, would probably be better off as a `FIXED` !
##########
File path: lang/java/avro/src/main/java/org/apache/avro/logicaltypes/AvroBoolean.java
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.logicaltypes;
+
+import org.apache.avro.AvroTypeException;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+
+/**
+ * Wrapper for the Avro BOOLEAN type.
+ */
+public class AvroBoolean implements AvroPrimitive {
+ public static final String NAME = "BOOLEAN";
+ private static final AvroBoolean ELEMENT = new AvroBoolean();
+ private static final Schema SCHEMA = Schema.create(Type.BOOLEAN);
+
+ private AvroBoolean() {
+ super();
+ }
+
+ public static AvroBoolean create() {
+ return ELEMENT;
+ }
+
+ @Override
+ public String toString() {
+ return NAME;
+ }
+
+ @Override
+ public Boolean convertToRawType(Object value) {
+ if (value == null) {
+ return null;
+ } else if (value instanceof Boolean) {
+ return (Boolean) value;
+ } else if (value instanceof String) {
+ if ("TRUE".equalsIgnoreCase((String) value)) {
Review comment:
Throughout the PR, these conversion rules are a bit surprising. Are they aligned to a standard?
It kind of doesn't matter which rules we pick, some developer will think it's unexpected, and this should be well-documented.
##########
File path: lang/java/avro/src/main/java/org/apache/avro/LogicalTypes.java
##########
@@ -222,6 +267,19 @@ private Decimal(Schema schema) {
}
}
+ protected Decimal(String text) {
Review comment:
For initialising from a string like DECIMAL(XX,YY) ?
##########
File path: lang/java/avro/src/main/java/org/apache/avro/logicaltypes/AvroBytes.java
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.logicaltypes;
+
+import java.nio.ByteBuffer;
+
+import org.apache.avro.AvroTypeException;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+
+/**
+ * Is the Avro Type.BYTES datatype, a binary store of any length. A BLOB column.
+ *
+ */
+public class AvroBytes implements AvroPrimitive {
+ public static final String NAME = "BYTES";
+ private static final AvroBytes ELEMENT = new AvroBytes();
+ private static final Schema SCHEMA = Schema.create(Type.BYTES);
+
+ private AvroBytes() {
+ super();
+ }
+
+ public static AvroBytes create() {
+ return ELEMENT;
+ }
+
+ @Override
+ public String toString() {
+ return NAME;
+ }
+
+ @Override
+ public ByteBuffer convertToRawType(Object value) {
+ if (value == null) {
+ return null;
+ } else if (value instanceof ByteBuffer) {
+ return (ByteBuffer) value;
+ } else if (value instanceof byte[]) {
+ return ByteBuffer.wrap((byte[]) value);
+ } else if (value instanceof Number) {
+ byte[] b = new byte[1];
Review comment:
To me, this is also a surprising conversion choice.
##########
File path: lang/java/avro/src/main/java/org/apache/avro/logicaltypes/AvroDatatype.java
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.logicaltypes;
+
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+
+/**
+ * This interface defines what information an Avro data type should provide and
+ * defines default conversion routines.
+ *
+ */
+public interface AvroDatatype {
+
+ /**
+ * Converts an input value into the object used by the backing data type. For
+ * example the data type is a timestamp-millis, which is backed by a Long. This
+ * method will convert any suitable input, e.g. Long, Instant,... into a long
+ * value required by the writer.
+ *
+ * @param value is the provided input
+ * @return an object in the correct backing data type
+ */
+ Object convertToRawType(Object value);
Review comment:
Should the output of this conversion be a generic? `<T>` It might be an opportunity to give a boost to type safety.
##########
File path: lang/java/avro/src/main/java/org/apache/avro/logicaltypes/AvroDate.java
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.logicaltypes;
+
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.ZoneId;
+import java.time.ZonedDateTime;
+import java.util.Date;
+
+import org.apache.avro.AvroTypeException;
+import org.apache.avro.LogicalTypes;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+import org.apache.avro.data.TimeConversions.DateConversion;
+
+/**
+ * Based on a Avro Type.INT holds the date portion without time.
+ *
+ */
+public class AvroDate extends LogicalTypes.Date implements AvroPrimitive {
+ public static final String NAME = "DATE";
+ public static final String TYPENAME = LogicalTypes.DATE;
+ private static final Schema SCHEMA;
+ private static final AvroDate ELEMENT = new AvroDate();
+ private static final DateConversion CONVERTER = new DateConversion();
+
+ static {
+ SCHEMA = ELEMENT.addToSchema(Schema.create(Type.INT));
+ }
+
+ private AvroDate() {
+ super();
+ }
+
+ public static AvroDate create() {
+ return ELEMENT;
+ }
+
+ @Override
+ public String toString() {
+ return NAME;
+ }
+
+ @Override
+ public Integer convertToRawType(Object value) {
+ if (value == null) {
+ return null;
+ } else if (value instanceof Integer) {
+ return (Integer) value;
+ } else if (value instanceof LocalDate) {
+ return CONVERTER.toInt((LocalDate) value, null, this);
+ } else if (value instanceof Number) {
+ return convertToRawType(((Number) value).intValue());
+ } else if (value instanceof CharSequence) {
+ return convertToRawType(LocalDate.parse((CharSequence) value));
Review comment:
This is almost certainly going to be unexpected to somebody (even if ISO_LOCAL_DATE is the least surprising of the formats)!
##########
File path: lang/java/avro/src/main/java/org/apache/avro/logicaltypes/AvroShort.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.logicaltypes;
+
+import org.apache.avro.AvroTypeException;
+import org.apache.avro.LogicalType;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+
+/**
+ * Based on an Avro Type.INT holds 2-byte signed numbers.
+ *
+ */
+public class AvroShort extends LogicalType implements AvroPrimitive {
+ public static final String NAME = "SHORT";
+ public static final String TYPENAME = NAME;
Review comment:
To this point, all of the logical types shipping with Avro come in lower-case-kebab style.
##########
File path: lang/java/avro/src/main/java/org/apache/avro/Schema.java
##########
@@ -565,6 +633,17 @@ public Field(String name, Schema schema) {
this(name, schema, (String) null, (JsonNode) null, true, Order.ASCENDING);
}
+ /**
+ * This is the data type of the field, e.g. if the field is defined as
+ * union[NULL, Long] the returned data type is AvroLong because the field should
+ * be set with Long values.
+ *
+ * @return the AvroDataType used to convert values for this field
+ */
+ public AvroDatatype getDataType() {
Review comment:
Minor nit-pick -- some inconsistent capital letters!
##########
File path: lang/java/avro/src/main/java/org/apache/avro/Schema.java
##########
@@ -191,6 +206,57 @@ void setLogicalType(LogicalType logicalType) {
this.logicalType = logicalType;
}
+ /**
+ * For the Avro-defined primitive and complex data types this method returns
+ * matching AvroDatatype objects. The Avro-provided logical data types
+ * themselves implement the AvroDatatype interface already.
+ *
+ * @return An object with more metadata about the data type and methods to
+ * convert values
+ */
+ public AvroDatatype getDataType() {
+ if (logicalType != null && logicalType instanceof AvroDatatype) {
+ return (AvroDatatype) logicalType;
Review comment:
So you can create new customized LogicalTypes and use them as long as they implement AvroDatatype.
##########
File path: lang/java/avro/src/main/java/org/apache/avro/logicaltypes/AvroType.java
##########
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.logicaltypes;
+
+import java.util.List;
+
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+import org.apache.commons.text.StringEscapeUtils;
+
+/**
+ * ENUM with the list of all Avro data types
+ *
+ */
+public enum AvroType {
+ /**
+ * A 8bit signed integer
+ */
+ AVROBYTE,
+ /**
+ * ASCII text of large size, comparison and sorting is binary
+ */
+ AVROCLOB,
+ /**
+ * Unicode text of large size, comparison and sorting is binary
+ */
+ AVRONCLOB,
+ /**
+ * Unicode text up t n chars long, comparison and sorting is binary
+ */
+ AVRONVARCHAR,
+ /**
+ * A 16bit signed integer
+ */
+ AVROSHORT,
+ /**
+ * A Spatial data type in WKT representation
+ */
+ AVROSTGEOMETRY,
+ /**
+ * A Spatial data type in WKT representation
+ */
+ AVROSTPOINT,
+ /**
+ * A string as URI
+ */
+ AVROURI,
+ /**
+ * An ASCII string of n chars length, comparison and sorting is binary
+ */
+ AVROVARCHAR,
+ /**
+ * A date without time information
+ */
+ AVRODATE,
+ /**
+ * A numeric value with precision and scale
+ */
+ AVRODECIMAL,
+ /**
+ * A time information down to milliseconds
+ */
+ AVROTIMEMILLIS,
+ /**
+ * A time information down to microseconds
+ */
+ AVROTIMEMICROS,
+ /**
+ * A timestamp down to milliseconds in UTC
+ */
+ AVROTIMESTAMPMILLIS,
+ /**
+ * A timestamp down to microseconds in UTC
+ */
+ AVROTIMESTAMPMICROS,
+ /**
+ * A timestamp down to milliseconds without time zone info
+ */
+ AVROLOCALTIMESTAMPMILLIS,
+ /**
+ * A timestamp down to microseconds without time zone info
+ */
+ AVROLOCALTIMESTAMPMICROS,
+ /**
+ * Boolean
+ */
+ AVROBOOLEAN,
+ /**
+ * A 32bit signed integer value
+ */
+ AVROINT,
+ /**
+ * A 64bit signed integer value
+ */
+ AVROLONG,
+ /**
+ * A 32bit floating point number
+ */
+ AVROFLOAT,
+ /**
+ * A 64bit floating point number
+ */
+ AVRODOUBLE,
+ /**
+ * Binary data of any length
+ */
+ AVROBYTES,
+ /**
+ * A unbounded unicode text - prefer using nvarchar or nclob instead to indicate
+ * its normal length, comparison and sorting is binary
+ */
+ AVROSTRING,
+ /**
+ * A binary object with an upper size limit
+ */
+ AVROFIXED,
+ /**
+ * A unicode string with a list of allowed values - one of enum(), comparison
+ * and sorting is binary
+ */
+ AVROENUM,
+ /**
+ * A unicode string array with a list of allowed values - many of map(),
+ * comparison and sorting is binary
+ */
+ AVROMAP,
+ /**
+ * An ASCII string formatted as UUID, comparison and sorting is binary
+ */
+ AVROUUID,
+ /**
+ * An array of elements
+ */
+ AVROARRAY,
+ /**
+ * A Record of its own
+ */
+ AVRORECORD;
+
+ /**
+ * In case this schema is a union of null and something else, it returns the
+ * _something else_
+ *
+ * @param schema of the input
+ * @return schema without the union of null, in case it is just that. Does
+ * return an union in all other cases.
+ */
+ public static Schema getBaseSchema(Schema schema) {
+ if (schema == null) {
+ return null;
+ } else if (schema.getType() == Type.UNION) {
+ List<Schema> types = schema.getTypes();
+ if (types.size() == 2 && types.get(0).getType() == Type.NULL) {
Review comment:
NULL appearing first in a union is just a convention. It can be the second branch in a UNION
##########
File path: lang/java/avro/src/main/java/org/apache/avro/logicaltypes/AvroDecimal.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.logicaltypes;
+
+import java.math.BigDecimal;
+import java.math.RoundingMode;
+import java.nio.ByteBuffer;
+
+import org.apache.avro.AvroTypeException;
+import org.apache.avro.Conversions.DecimalConversion;
+import org.apache.avro.LogicalTypes;
+import org.apache.avro.LogicalTypes.Decimal;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+import org.apache.avro.generic.GenericFixed;
+
+public class AvroDecimal extends Decimal implements AvroPrimitive {
+ private static final DecimalConversion DECIMAL_CONVERTER = new DecimalConversion();
+ public static final String NAME = "DECIMAL";
+ public static final String TYPENAME = LogicalTypes.DECIMAL;
+
+ public AvroDecimal(String text) {
+ super(text);
+ }
+
+ public AvroDecimal(int precision, int scale) {
+ super(precision, scale);
+ }
+
+ public static AvroDecimal create(int precision, int scale) {
+ return new AvroDecimal(precision, scale);
+ }
+
+ public AvroDecimal(Schema schema) {
+ super(schema);
+ }
+
+ @Override
+ public String toString() {
+ return NAME + "(" + getPrecision() + "," + getScale() + ")";
+ }
+
+ @Override
+ public Schema getRecommendedSchema() {
+ return addToSchema(Schema.create(Type.BYTES));
Review comment:
This can also be a FIXED -- is there any reason to recommend one over the other? Could we have two AvroDecimal?
##########
File path: lang/java/avro/src/main/java/org/apache/avro/logicaltypes/AvroDatatype.java
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.logicaltypes;
+
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+
+/**
+ * This interface defines what information an Avro data type should provide and
+ * defines default conversion routines.
+ *
+ */
+public interface AvroDatatype {
+
+ /**
+ * Converts an input value into the object used by the backing data type. For
+ * example the data type is a timestamp-millis, which is backed by a Long. This
+ * method will convert any suitable input, e.g. Long, Instant,... into a long
+ * value required by the writer.
+ *
+ * @param value is the provided input
+ * @return an object in the correct backing data type
+ */
+ Object convertToRawType(Object value);
+
+ /**
+ * @return the Avro schema type used for storing this logical type
+ */
+ Type getBackingType();
+
+ /**
+ * @return a default schema for this data type for primitives, null otherwise
+ */
+ Schema getRecommendedSchema();
Review comment:
Where is this used? Why is it recommended? (I noticed that the decimal logical type recommends bytes, but it can be bytes or fixed).
I've never seen a logical type on an aggregate type (like a UNION or a RECORD), but I think it's meant to be possible.
##########
File path: lang/java/avro/src/main/java/org/apache/avro/logicaltypes/AvroEnum.java
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.logicaltypes;
+
+import org.apache.avro.AvroTypeException;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+import org.apache.avro.generic.GenericData.EnumSymbol;
+import org.apache.avro.generic.GenericEnumSymbol;
+
+/**
+ * Wrapper around the Avro Type.ENUM data type
+ *
+ */
+public class AvroEnum implements AvroPrimitive {
+ public static final String NAME = "ENUM";
+ private final Schema schema;
+
+ public AvroEnum(Schema schema) {
+ super();
+ this.schema = schema;
+ }
+
+ public static AvroEnum create(Schema schema) {
+ return new AvroEnum(schema);
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o)
+ return true;
+ if (o == null || getClass() != o.getClass())
+ return false;
+ if (schema != null) {
+ if (((AvroEnum) o).getRecommendedSchema() != null) {
+ return schema.equals(((AvroEnum) o).getRecommendedSchema());
+ }
+ } else if (((AvroEnum) o).getRecommendedSchema() == null) {
+ return true;
+ }
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ if (schema != null) {
+ return schema.hashCode();
Review comment:
I'm not sure I understand under what circumstances this schema can be null.
##########
File path: lang/java/avro/src/main/java/org/apache/avro/logicaltypes/AvroBoolean.java
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.logicaltypes;
+
+import org.apache.avro.AvroTypeException;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+
+/**
+ * Wrapper for the Avro BOOLEAN type.
+ */
+public class AvroBoolean implements AvroPrimitive {
+ public static final String NAME = "BOOLEAN";
+ private static final AvroBoolean ELEMENT = new AvroBoolean();
+ private static final Schema SCHEMA = Schema.create(Type.BOOLEAN);
+
+ private AvroBoolean() {
+ super();
+ }
+
+ public static AvroBoolean create() {
+ return ELEMENT;
+ }
+
+ @Override
+ public String toString() {
+ return NAME;
+ }
+
+ @Override
+ public Boolean convertToRawType(Object value) {
+ if (value == null) {
+ return null;
+ } else if (value instanceof Boolean) {
+ return (Boolean) value;
+ } else if (value instanceof String) {
+ if ("TRUE".equalsIgnoreCase((String) value)) {
+ return Boolean.TRUE;
+ } else if ("FALSE".equalsIgnoreCase((String) value)) {
+ return Boolean.FALSE;
+ }
+ } else if (value instanceof Number) {
+ int v = ((Number) value).intValue();
+ if (v == 1) {
+ return Boolean.TRUE;
+ } else if (v == 0) {
+ return Boolean.FALSE;
+ }
+ }
+ throw new AvroTypeException(
+ "Cannot convert a value of type \"" + value.getClass().getSimpleName() + "\" into a Boolean");
Review comment:
This could say `Cannot convert a value of type "String" into a Boolean`
For AvroByte, for example, specific nonconvertible values are printed.
##########
File path: lang/java/avro/src/main/java/org/apache/avro/logicaltypes/AvroDatatype.java
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.logicaltypes;
+
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+
+/**
+ * This interface defines what information an Avro data type should provide and
+ * defines default conversion routines.
+ *
+ */
+public interface AvroDatatype {
Review comment:
For consistency: `AvroDataType`? If we could come up with a more explicit name, that would be better! I mean, this is specifically responsible for doing to logical type conversions and providing more information about the logical type. If `Conversion` didn't already exist, it would be good candidate for a name.
##########
File path: lang/java/avro/src/main/java/org/apache/avro/logicaltypes/AvroDatatype.java
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.logicaltypes;
+
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
+
+/**
+ * This interface defines what information an Avro data type should provide and
+ * defines default conversion routines.
+ *
+ */
+public interface AvroDatatype {
+
+ /**
+ * Converts an input value into the object used by the backing data type. For
+ * example the data type is a timestamp-millis, which is backed by a Long. This
+ * method will convert any suitable input, e.g. Long, Instant,... into a long
+ * value required by the writer.
+ *
+ * @param value is the provided input
+ * @return an object in the correct backing data type
+ */
+ Object convertToRawType(Object value);
+
+ /**
+ * @return the Avro schema type used for storing this logical type
+ */
+ Type getBackingType();
Review comment:
There's a question of consistent terminology here -- sometimes it's **"raw"** type and sometimes it's **"backing"** type.
I have typically used **"underlying"** type (or "primitive" type if I can be sure that the type underneath is a primitive). I like either of your terms better, but we should try to be consistent thorught the code.
##########
File path: lang/java/avro/src/main/java/org/apache/avro/Schema.java
##########
@@ -565,6 +633,17 @@ public Field(String name, Schema schema) {
this(name, schema, (String) null, (JsonNode) null, true, Order.ASCENDING);
}
+ /**
+ * This is the data type of the field, e.g. if the field is defined as
+ * union[NULL, Long] the returned data type is AvroLong because the field should
+ * be set with Long values.
Review comment:
This is also true for any UNION schema -- shouldn't `getField(0).getDataType()` here always be equivalent to `getField(0).schema().getDataType()`?
----------------------------------------------------------------
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
[GitHub] [avro] unchuckable commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
unchuckable commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-744058529
> Before I make the changes, any counter arguments? Any requests from the Avro maintainers?
Didn't mean to dishearten BTW, just give the honest feedback ;)
----------------------------------------------------------------
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
[GitHub] [avro] emkornfield commented on a change in pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #973:
URL: https://github.com/apache/avro/pull/973#discussion_r586118999
##########
File path: lang/java/avro/src/main/java/org/apache/avro/file/DataFileReader.java
##########
@@ -58,7 +58,7 @@
// read magic header
byte[] magic = new byte[MAGIC.length];
in.seek(0);
- for (int c = 0; c < magic.length; c += in.read(magic, c, magic.length - c)) {
+ for (int c = 0; c < magic.length; c = in.read(magic, c, magic.length - c)) {
Review comment:
is this change intended?
----------------------------------------------------------------
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
[GitHub] [avro] wernerdaehn commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
wernerdaehn commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-744052343
* **the methods public String `toString()` and `public String toString(StringBuilder, Value)` imply they do something similar, but actually don't:** Correct. But as both methods do not make sense by themselves, both help only in online debugging. I have no problem change the name, though, your choice.
* **the methods `public int hashCode()` are not implemented consistently, and where they are, they all return 1 which leads to bad hashmapping in cases where that might ever be used**: There are two types of LogicalTypes: 1) Types like AvroInt, AvroLong etc that do not need any parameters. Hence they are Singletons. You cannot create a new one, you use the factory method to return the ONE. As there is just a single object, the hashCode() can return anything. Case 2) is where different instances are created because e.g. an AvroVarchar has a length. All these classes have a proper hashCode, e.g. LogicalTypeWithLength returns the hashCode of the length. So here I believe we are fine.
* **`public boolean equals()` feels odd where it addresses Arrays and Maps**: I assume you meant Enum and Map. When should Enum.equals(Enum) return true? If both are Enums or if both are Enums and have the exact schema? Or if both are Enums and the schemas are forward compatible? I believe the first is the best option, as you will use these comparisons to convert values. And for values the possible values do not play a role. Hence I would suggest to stick with the current logic: all Enum datatypes are Enum datatypes.
* **instances of override functions that only call their super version are redundant**: True. I thought it is a good idea for readability. To remind myself that there is nothing special. I have no problem changing that.
* **it is unclear to me why some effectively static final fields are only set as static**: That I can explain, I am lacy on defining things as final. Agree, that is something I should change.
* **`getRecommendedSchema` returns null in some cases (arrays, maps, records), but neither interface or javadoc specify that**: Deal, will update the Javadoc.
* **getBackingType, getRecommendedSchema**: I took some inspiration from the current LogicalTypes, there the classes are rather self sufficient as well. And actually, it does make sense. When something does change and it should return a different object, a one-line change, you do not want to modify code at three different classes. Hence my feeling is it should rather stay as is. But maybe I misunderstood your point. Do you have an example?
Before I make the changes, any counter arguments? Any requests from the Avro maintainers?
----------------------------------------------------------------
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
[GitHub] [avro] RyanSkraba commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
RyanSkraba commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-773369646
Hello! I haven't had the time to attack this PR. There's a lot of lines of code, and thanks so much to @unchuckable for picking up the slack on the peer review -- great job, your opinion is welcomed and valued! @wernerdaehn of course, I also appreciate your openness, patience and willingness to contribute your work!
I have some concerns about the interactions between the new AvroDatatype interface being introduced and the existing Conversions, as well as existing non-standard user-defined LogicalTypes -- I really need to give this a thorough read to better understand. With my time constraints, I can commit to doing a read through and discussion in the next week!
I would love to have any other committers weigh in, especially if anyone has any ideas or experience on making big changes less daunting. I don't want to make *more* work for you!
----------------------------------------------------------------
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
[GitHub] [avro] RyanSkraba commented on a change in pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
RyanSkraba commented on a change in pull request #973:
URL: https://github.com/apache/avro/pull/973#discussion_r586254399
##########
File path: lang/java/avro/src/main/java/org/apache/avro/file/DataFileReader.java
##########
@@ -58,7 +58,7 @@
// read magic header
byte[] magic = new byte[MAGIC.length];
in.seek(0);
- for (int c = 0; c < magic.length; c += in.read(magic, c, magic.length - c)) {
+ for (int c = 0; c < magic.length; c = in.read(magic, c, magic.length - c)) {
Review comment:
It looks like this change doesn't belong to any commits from this PR. An artifact from Github from changes on master since this PR was proposed?
I played with the "Resolve conflicts" directly on github for the first time, and it made a commit directly to your master branch -- my apologies, I'm not sure what I was expecting!
----------------------------------------------------------------
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
[GitHub] [avro] wernerdaehn commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
wernerdaehn commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-789455181
You are right, this is a copy/paste from another discussion. Should be removed from here.
----------------------------------------------------------------
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
[GitHub] [avro] wernerdaehn commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
wernerdaehn commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-744184926
(And I never said that all my contributions are flawless without a doubt. It is also my first contribution to Avro, hence I have no feel for it yet)
----------------------------------------------------------------
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
[GitHub] [avro] RyanSkraba commented on pull request #973: AVRO-2952: AvroDatatype
Posted by GitBox <gi...@apache.org>.
RyanSkraba commented on pull request #973:
URL: https://github.com/apache/avro/pull/973#issuecomment-773369646
Hello! I haven't had the time to attack this PR. There's a lot of lines of code, and thanks so much to @unchuckable for picking up the slack on the peer review -- great job, your opinion is welcomed and valued! @wernerdaehn of course, I also appreciate your openness, patience and willingness to contribute your work!
I have some concerns about the interactions between the new AvroDatatype interface being introduced and the existing Conversions, as well as existing non-standard user-defined LogicalTypes -- I really need to give this a thorough read to better understand. With my time constraints, I can commit to doing a read through and discussion in the next week!
I would love to have any other committers weigh in, especially if anyone has any ideas or experience on making big changes less daunting. I don't want to make *more* work for you!
----------------------------------------------------------------
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