morningman commented on code in PR #68921:
URL: https://github.com/apache/doris/pull/68921#discussion_r4269728398
##########
fe/fe-core/src/main/java/org/apache/doris/connector/ConnectorSessionImpl.java:
##########
@@ -165,6 +166,12 @@ public Map<String, String> getSessionProperties() {
return sessionProperties;
}
+ @Override
Review Comment:
Fixed in f58089e1cf1. My earlier reply no longer holds: #68837, merged after
it, pins one execution version per statement
(`NereidsPlanner.pinBeExecVersion`). The query options sent to the backends,
the write plan context (which
`PhysicalConnectorTableSink.requireExternalWriterRoutingSupport` checks) and
the Variant gate in `PluginDrivenTableSink` all read that pinned value. As your
later rounds noted, the new session method was the only one still reading the
live `Config.be_exec_version`, so its javadoc claim that it carries the same
value as the write handle was no longer true.
`ConnectorSessionBuilder` now captures the version pinned on the statement
context, the same context it takes the statement scope from, and
`ConnectorSessionImpl#getBeExecVersion()` returns it. A session built without a
statement context, or before the planner pins the version, still reports
`Config.be_exec_version`. The only caller,
`IcebergWritePlanProvider.getWriteDistribution`, runs from
`PluginDrivenExternalTable.resolveWritePlanContext()`, which already requires
the pinned version for the write plan context.
Tests:
`ConnectorSessionImplTest.testBeExecVersionIsTheVersionPinnedForTheStatement`
pins 17, sets the config to 18 and expects 17; reading the config instead fails
it. `testBeExecVersionFallsBackToTheEngineConfigOutsideAPlannedStatement`
covers the fallback.
--
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]
