Support CREATE VIEW and CREATE TABLE verification #15195
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
caithagoras
force-pushed
the
s1
branch
17 times, most recently
from
382eee7
to
1fc9474
Compare
caithagoras
changed the title
caithagoras
force-pushed
the
s1
branch
2 times, most recently
from
3f0ae5d
to
5c375eb
Compare
|
started reviewing - first 3 commits LGTM |
caithagoras
force-pushed
the
s1
branch
2 times, most recently
from
219e908
to
53f80a8
Compare
sujay-jain
reviewed
Sep 30, 2020
| protected Optional<String> resolveFailure(Optional<QueryObjectBundle> control, Optional<QueryObjectBundle> test, QueryContext controlQueryContext, Optional<DdlMatchResult> matchResult, Optional<Throwable> throwable) | ||
| { | ||
| return Optional.empty(); | ||
| } |
There was a problem hiding this comment.
Do we not want a failureResolveManager like DataVerification?
There was a problem hiding this comment.
Failure resolver is used to automatically resolve verification failures due to (test / checksum) query failures. For ddl verification, we can start with no auto resolution, and see if we actually see any query failures are actually false alerts.
sujay-jain
reviewed
Sep 30, 2020
| // Otherwise, do not pre-create temporary view. | ||
| try { | ||
| CreateView createExistingView = (CreateView) sqlParser.createStatement( | ||
| getOnlyElement(prestoAction.execute(new ShowCreate(VIEW, createView.getName()), REWRITE, SHOW_CREATE_VIEW_CONVERTER).getResults()), |
There was a problem hiding this comment.
Should we extract this into a method? Feel like it would be easier to follow
There was a problem hiding this comment.
This method would only be used once if we extract. I'm changing this to 2 statements - that might help readability.
Remove unsupported query types in QueryType. Also, remove QueryType.Category.
Repurpose the class to represent query bundle with any object name, including table name and view name.
If either all the failure resolvers or all the failure resolver factories are disabled, injection error would occurred due to Set<FailureResolver> or Set<FailureResolverFactory> not binded. Also, extract common logic of verification tests into an abstract tests.
If the specified view already exists, create a temporary view to match the existing view. Otherwise, do nothing for setup queries. Rewrite the target view name, and run both control and test queries. Run a SHOW CREATE VIEW query and the returned CREATE VIEW statement needs to match.
Rewrite the target table of the CREATE TABLE statement, run both control and test queries, run SHOW CREATE TABLE query as the check.
|
@sujay-jain Thanks for reviewing! |
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Verify CREATE VIEW
If the specified view already exists, create a temporary view to match
the existing view. Otherwise, do nothing for setup queries. Rewrite the
target view name, and run both control and test queries. Run a SHOW
CREATE VIEW query and the returned CREATE VIEW statement needs to match.
Verify CREATE TABLE
Rewrite the target table of the CREATE TABLE statement, run both
control and test queries, run SHOW CREATE TABLE query as the check.