[GitHub] [avro] RyanSkraba commented on a change in pull request #1411: AVRO-3257: IDL support for nullable types

[GitBox <gi...@apache.org>]

RyanSkraba commented on a change in pull request #1411:
URL: https://github.com/apache/avro/pull/1411#discussion_r766035699



##########
File path: lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj
##########
@@ -182,6 +183,32 @@ public class Idl implements Closeable {
     return result;
   }
 
+  /**
+   * For "optional schemas" (recognized by the marker property the NullableType
+   * production adds), ensure the null schema is in the right place.
+   *
+   * @param schema       a schema
+   * @param defaultValue the intended default value
+   * @return the schema, or an optional schema with null in the right place
+   */
+  private static Schema fixOptionalSchema(Schema schema, JsonNode defaultValue) {

Review comment:
       I like this so much!  This is a compelling feature... I'm tempted to say that nobody really **_wants_**
   ```
   union { null, double } value = NaN
   ```
   to fail either...  What would you think about auto-correcting all `union { null, notNull }` or `union { notNull, null } ` types that have a default?
   
   I guess this might be too magical, and i suspect that the `?` syntax will be happily adopted in any case.




-- 
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: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org