feat: allow for only Gen 1 create or replace queries with server level configs via rest api, bars all gen 2 queries #8711
Conversation
…el configs via rest api
|
@rodesai This is the check we talked about that should allow for server level configs to be set for old queries during a create or replace. But it will make sure that any queries going into the new runtimes will fail with a useful message |
|
When testing this PR I found an issue with recovering replaced gen 1 queries that I fixed here #8713 I will hold off merging until that is merged |
There was a problem hiding this comment.
Hi @wcarlson5 the title and the description and the content of the PR seems to be indicating a bit different scopes: from the title and the content of the PR, we are only checking against the create or replace query stmt to not have any server level config overrides; whereas in your PR description you mentioned This forces any queries coming into the shared runtimes.. could you modify it to be consistent and hence easy to understand fo other readers?
Otherwise, LGTM!
wcarlson5
changed the title
wcarlson5
changed the title
| @@ -272,6 +273,7 @@ public PersistentQueryMetadata createOrReplacePersistentQuery( | |||
|
|
|||
| if (sharedRuntimeId.isPresent() && (oldQuery == null | |||
| || oldQuery instanceof BinPackedPersistentQueryMetadataImpl)) { | |||
| throwOnNonQueryLevelConfigs(config.getOverrides()); | |||
There was a problem hiding this comment.
Sorry for the late feedback here. We should only do validation like this in the validation phase of executing a statement. Otherwise we might change the validation later on in a way that causes us to fail statements we've already accepted. The pattern for doing that is to do this type of validation in the sandbox instance only - can you make that change in a follow-up?
There was a problem hiding this comment.
#8722 sure
Those configs will be ignored but it won't mean there is a record in the cmd topic that will cause it to fail.
This forces any queries coming into the shared runtimes to not have overridden the server level props. If the query will not be going into the new shared runtimes (i.e. is a
create or replaceof an old query) it will not be held to those standards.Testing done
Describe the testing strategy. Unit and integration tests are expected for any behavior changes.
Reviewer checklist