remove CONTINUATION_CONTAINS_COMPILED_STATEMENT usage #3650
Conversation
g31pranjal
force-pushed
the
remove_complied_statement_option
branch
from
b0b2ada to
23d49b7
Compare
There was a problem hiding this comment.
LGTM. I have a PR to bump the version to 4.7: #3653. It might be good to bundle that in with the other 4.7 changes as a kind of API change that we're delivering here
| static com.apple.foundationdb.relational.jdbc.grpc.v1.Options.Builder toProtobuf(@Nonnull Options options) throws SQLException { | ||
| final var builder = com.apple.foundationdb.relational.jdbc.grpc.v1.Options.newBuilder(); | ||
| // Switched-on by default on JDBC driver until the option is deprecated and removed. | ||
| builder.setContinuationsContainCompiledStatements(true); |
There was a problem hiding this comment.
Is the idea here that we need to set this in case the Options from a modern version are shipped to a older server that expects the field to be set?
There was a problem hiding this comment.
The idea, from this version, is that the option cannot be explicitly set to False. Setting it to TRUE here is indicative of what we are getting into starting now. Also, the check here is to ensure that we never set it to False going ahead, or to maintain backward compatibility. After this is rolled out, we need a PR to remove usage of this option in JDBC and mark it deprecated. Then, we remove the field itself!
This PR does a couple of things:
CONTINUATION_CONTAINS_COMPILED_STATEMENToption in Connection and as a query option.SELECT ... WITH CONTINUATION ..., which could be removed separately after this work. Hence, it could happen that a query started with a non-serialized continuation, but has now on serialised continuation.TRUEand rejects connections that have it set toFALSE. Once this is rolled out, we should probably stop failing on false and mark the option as deprecated in proto. Finally, we can stop setting the option and remove the option altogether.