danny0405 commented on a change in pull request #13763:
URL: https://github.com/apache/flink/pull/13763#discussion_r513370899



##########
File path: 
flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/typeutils/AvroSchemaConverter.java
##########
@@ -297,17 +299,22 @@ private static DataType convertToDataType(Schema schema) {
         * @return Avro's {@link Schema} matching this logical type.
         */
        public static Schema convertToSchema(LogicalType logicalType) {
-               return convertToSchema(logicalType, "record");
+               return convertToSchema(logicalType, "record", true);
        }
 
        /**
         * Converts Flink SQL {@link LogicalType} (can be nested) into an Avro 
schema.
         *
         * @param logicalType logical type
         * @param rowName     the record name
+        * @param top         whether it is parsing the root record,
+        *                    if it is, the logical type nullability would be 
ignored
         * @return Avro's {@link Schema} matching this logical type.
         */
-       public static Schema convertToSchema(LogicalType logicalType, String 
rowName) {
+       public static Schema convertToSchema(
+                       LogicalType logicalType,
+                       String rowName,
+                       boolean top) {

Review comment:
       I had offline discussion with @wuchong and @dawidwys , and after some 
research, we found that a non-nullable row type is more reasonable.
   
   But because the change is huge(many codes that convert a type info to data 
type assumes nullable true before), me and @wuchong decide to change the method 
signature to `convertToSchema(RowType schema)` and add a notion to the method 
doc that the passed in `schema` must be the top level record type.




----------------------------------------------------------------
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]


Reply via email to