ankitsultana commented on PR #10423: URL: https://github.com/apache/pinot/pull/10423#issuecomment-1533258014
> > My main concern is with this part: > > ``` > > builder.add(e.getKey(), toRelDataType(e.getValue(), > > isNullSupportEnabled || e.getValue().isNullableField())); > > ``` > > > > > > > > > > > > > > > > > > > > > > > > I think if users have configured "enableNullHandling" at table level, we should let column level nullability decide whether a given column is nullable or not. > > If enableNullHandling is not set at the table level, then we can assume that all columns are non-null (similar to V1 engine). That would essentially mean that column level nullability is not enabled so we assume everything to be non-null. > > Hmm. I understood your concern. so to link: https://github.com/apache/pinot/pull/10423/files#r1183232264 --> this means in order to enable nullability we should > > 1. only do so for a table if table config enables null, otherwise disregard any column level config (this is existing behavior) > 2. add column level nullability config to user setting it to false, otherwise default to true (this is going to change) > 3. create nullable column only if (1) _AND_ (2) are true > > yes? Yeah. The idea being that we only have two states: 1. Either column level nullability feature is disabled (when enableNullHandling is not set at table level) 2. Or it is enabled, in which case its values will always be honored. -- 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]
