924060929 commented on PR #68320:
URL: https://github.com/apache/doris/pull/68320#issuecomment-5865847836

   I think the connector/FE/BE responsibility split is sound, but the 
write-planning contract still needs one consistent metadata snapshot.
   
   `PhysicalConnectorTableSink.getRequirePhysicalProperties()` calls 
`PluginDrivenExternalTable.getConnectorWriteDistribution()`, which resolves a 
fresh table handle/provider. The latter returns `Optional.empty()` both when 
the provider declares no custom distribution and when that handle/provider 
cannot be resolved. The physical plan then falls back to generic distribution. 
Later, `PhysicalPlanTranslator.visitPhysicalConnectorTableSink()` resolves the 
handle/provider again to build the sink. Thus the exchange routing and sink can 
be planned from different table metadata generations; a transient miss in the 
first lookup can also silently remove a required routing contract if the later 
lookup succeeds.
   
   For an external-hash writer, a change in bucket count or partition-function 
options between those lookups could send rows according to one routing 
definition while the sink is built for another. I am flagging this as a 
framework contract risk rather than a reproduced issue in an existing 
connector: this PR does not yet have a connector implementation of the new 
distribution SPI.
   
   Could we pin the handle/provider and distribution together with the bound 
write schema/metadata identity, then carry that statement-scoped description 
through property derivation and sink construction? An unresolved required 
routing lookup should fail instead of being treated as "no custom 
distribution". A test that changes the provider's routing metadata between 
planning stages would make this guarantee explicit.
   


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

Reply via email to