Skip to content

Conversation

@caithagoras
Copy link
Contributor

@caithagoras caithagoras commented Sep 1, 2020

Add a new functionality to the verifier to allow explain verification.

For each query pairs to be verified, explain both control and test, and
compare the query plans. Verification is marked as succeeded as long as
the test query can be explained.

Introduced a new field matchType in the output event, which can be used
to indicate whether there is plan difference.

For non-DML queries, we don't care much about plan difference and hence
the control query and the plan comparison are skipped.

== RELEASE NOTES ==

Verifier Changes
* Add support to run explain verification. (:pr:`15101`). This can be enabled by configuration property ``explain``.

@caithagoras caithagoras changed the title [WIP Support explain verification [WIP] Support explain verification Sep 1, 2020
@caithagoras caithagoras force-pushed the s1 branch 12 times, most recently from 06c39a0 to 1811342 Compare September 1, 2020 04:58
@caithagoras caithagoras changed the title [WIP] Support explain verification Support explain verification Sep 1, 2020
@caithagoras caithagoras force-pushed the s1 branch 7 times, most recently from 9879ca9 to 10ad3bf Compare September 2, 2020 00:21
@caithagoras caithagoras requested review from sujay-jain and removed request for bhhari, tdcmeehan and yingsu00 September 3, 2020 09:14
@caithagoras caithagoras force-pushed the s1 branch 2 times, most recently from 8d87536 to 340ecf9 Compare September 8, 2020 04:57
@sujay-jain
Copy link
Member

Still reviewing - first 2 commits LGTM

@caithagoras caithagoras force-pushed the s1 branch 5 times, most recently from 3e07ba0 to 5dd8d16 Compare September 10, 2020 09:43
Copy link
Member

@sujay-jain sujay-jain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@caithagoras
Copy link
Contributor Author

caithagoras commented Sep 10, 2020

@mbasmanova This PR is ready now. Thank you for helping!

Convert MatchResult to an interface and move the data-verification-
specific implementation of the MatchResult to DataMatchResult.
QueryBundle contained a field tableName, but not all query rewrites
result in queries with a target table. Move the field to a subclass
DataQueryBundle.
Add a new functionality to the verifier to allow explain verification.

For each query pairs to be verified, explain both control and test, and
compare the query plans. Verification is marked as succeeded as long as
the test query can be explained.

Introduced a new field matchType in the output event, which can be used
to indicate whether there is plan difference.

For non-DML queries, we don't care much about plan difference and hence
the control query and plan comparison are skipped.
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.

3 participants