DanielLeens commented on PR #11216: URL: https://github.com/apache/seatunnel/pull/11216#issuecomment-4893562452
Thanks for catching this — I agree with your compatibility concern. I rechecked the current diff around `MilvusSinkConverter.buildMilvusData(...)`, and the dynamic-field branch now runs before the generic null validation. In the old flow, a `null` value was rejected before the dynamic-field parsing branch, so a null dynamic-field column also raised `MILVUS-10`. In the current flow, when `enable_dynamic_field=true` and the dynamic-field value is `null`, the code enters the dynamic-field branch, skips adding anything, and `continue`s before the normal null validation can run. That means this changes default behavior even when `enable_nullable_field=false`, and `skipsNullDynamicFieldBeforeNullValidation` currently codifies that behavior. I do not think my previous approval should stand with this compatibility gap open. Could you please restore the old default behavior for null dynamic-field values, and only allow the skip/null handling if it is explicitly covered by the new nullable-field contract? After that update I’m happy to re-review the latest head. -- 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]
