Repository navigation
fix: validate select items and skipped clauses in SelectValidator - #2879
Open
tanjiroK-coder wants to merge 1 commit into
Open
tanjiroK-coder wants to merge 1 commit into
tanjiroK-coder wants to merge 1 commit into
Conversation
SelectValidator never validated the select list (the call was commented out), nor DISTINCT ON, TOP, QUALIFY, WINDOW definitions and the LIMIT, OFFSET and FETCH row counts, so no capability saw the expressions there. They are now validated like WHERE, with the select list after the FROM items so table aliases resolve.
This branch has not been deployed
This file contains hidden or 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
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.
SelectValidator.visit(PlainSelect)never validated the select list: the call was commented out with a@todoabout aliased tables, so nothing in it reachedExpressionValidatorand noValidationCapabilitysaw it.DISTINCT ON,TOP,QUALIFY, namedWINDOWdefinitions and theLIMIT/OFFSET/FETCHrow counts were skipped the same way (LimitValidatorandvalidateOffsetonly reported features,validateFetchonly looked at a JDBC parameter). WithFeaturesAllowed.SELECT.copy().remove(Feature.function),SELECT 1 FROM t WHERE pg_read_file('/etc/passwd') IS NOT NULLreportsfunction not allowed.butSELECT pg_read_file('/etc/passwd')validates with no errors, and withFeaturesAllowed.SELECTaFOR UPDATEsubquery is rejected inWHEREbut accepted asSELECT (SELECT a FROM t FOR UPDATE). A few existing tests work around it by putting the expression inWHERE(testValidateFunctionArgumentInWhere,SqlServerXmlMethodTest.validatesMethodAsFunction).The select list,
DISTINCT ONandTOPare now validated right after the FROM items, so table aliases resolve as the old comment intended (testValidationMetadataSelectWithColumnsAndAlias2,select my.id from mytable as my, now really checksmy.idand still passes), and the other clauses go through the same helpers asWHEREandORDER BY. One visible effect forJdbcDatabaseMetaDataCapabilityusers: select-list columns are now looked up likeWHEREcolumns, so a column of a derived table (SELECT x.c FROM (SELECT ...) x) is reported there the wayWHERE x.c = 1already is. The new cases inSelectValidatorTest(11 clauses plus theFOR UPDATEsubquery) andDatabaseMetaDataValidationTestfail before the change and pass after;./gradlew checkandmvn verifypass.