[GitHub] [pinot] walterddr commented on pull request #10423: [multistage] proper support for column-level null handling

["walterddr (via GitHub)" <gi...@apache.org>]

walterddr commented on PR #10423:
URL: https://github.com/apache/pinot/pull/10423#issuecomment-1532448029

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


-- 
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: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org