Skip to content

Allow typedIn to run in replace-with-default mode.#16233

Merged
clintropolis merged 2 commits intoapache:masterfrom
gianm:typed-in-compat
Apr 4, 2024
Merged

Allow typedIn to run in replace-with-default mode.#16233
clintropolis merged 2 commits intoapache:masterfrom
gianm:typed-in-compat

Conversation

@gianm
Copy link
Copy Markdown
Contributor

@gianm gianm commented Apr 3, 2024

Useful when data servers, like Historicals, are running in replace-with-default mode and the Broker is running in SQL-compatible mode, which can happen during a rolling update that is applying a mode change.

Useful when data servers, like Historicals, are running in replace-with-default
mode and the Broker is running in SQL-compatible mode, which can happen during
a rolling update that is applying a mode change.
@gianm gianm added this to the 30.0.0 milestone Apr 3, 2024
@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Apr 3, 2024

I tested this locally by running a Broker in SQL-compatible mode and a Historical in replace-with-default mode. In this case, the Broker generates inType filters. Without this patch, the Historical threw an error on the query; with this patch, the query ran successfully.

@pranavbhole
Copy link
Copy Markdown
Contributor

CI is failing due to low branch coverage.

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Apr 3, 2024

I think we should ignore the branch coverage check here, because the relevant branch is if (NullHandling.sqlCompatible()), which we actually are testing both branches of— just not in the same run.

There were some legit test failures in the non-SQL-compat processing run, which I've fixed in the most recent commit.

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Apr 4, 2024

The legit failure is fixed; all that's left is the branch coverage failures, which I suggest we ignore.

@clintropolis clintropolis merged commit a319b44 into apache:master Apr 4, 2024
@gianm gianm deleted the typed-in-compat branch April 5, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants