[CALCITE-6219] Must Filter validation #3688
Closed
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
Added many tests and cleaned up Fix style, delete unused add return in case block Fix all checkstyle and add java docs Remove unused remove unused inport checkstyle spacing fix and forbiddenApi removal Update validateClause() checkstyleMain test fix Missing @OverRide annotation Add missing @OverRide Add missing Override annotation Change to static annotation
…ss all tests Next step is to remove AlwaysFilterValidatorImpl and fix the various TODO
olivrlee
changed the title
|
julianhyde
added a commit
to julianhyde/calcite
that referenced
this pull request
Feb 17, 2024
julianhyde
added a commit
to julianhyde/calcite
that referenced
this pull request
Feb 17, 2024
A table can declare that some of its columns must be filtered by implementing `interface SemanticTable`. If such columns are not filtered in a WHERE or HAVING clause, the validator throws. There are several purposes of these columns, one of which is to prevent expensive full-table scans (for example, reading all Orders without restricting on orderDate). Implementation is via the method SqlValidatorNamespace.getMustFilterFields(). For a table namespace, that method returns the declared must-filter fields. For a query namespce, that method returns any must-filter fields that have not been filtered in that query; such fields become the responsibility of the enclosing query. If a field is must-filter and is not in the SELECT clause of a sub-query, that is also an error, because of course it is now impossible for the enclosing query to filter it. Close apache#3688 Co-authored-by: Julian Hyde <jhyde@apache.org>
julianhyde
added a commit
to julianhyde/calcite
that referenced
this pull request
Feb 17, 2024
A table can declare that some of its columns must be filtered by implementing `interface SemanticTable`. If such columns are not filtered in a WHERE or HAVING clause, the validator throws. There are several purposes of these columns, one of which is to prevent expensive full-table scans (for example, reading all Orders without restricting on orderDate). Implementation is via the method SqlValidatorNamespace.getMustFilterFields(). For a table namespace, that method returns the declared must-filter fields. For a query namespce, that method returns any must-filter fields that have not been filtered in that query; such fields become the responsibility of the enclosing query. If a field is must-filter and is not in the SELECT clause of a sub-query, that is also an error, because of course it is now impossible for the enclosing query to filter it. Close apache#3688 Co-authored-by: Julian Hyde <jhyde@apache.org>
tanclary
reviewed
Feb 21, 2024
|
|
||
| if (validator.config().identifierExpansion()) { | ||
| SqlValidatorTable table = resolvedNamespace.getTable(); | ||
| if (table != null) { |
There was a problem hiding this comment.
It's hard to tell on github but could we remove a level of nesting if we change this to if (table == null) { break; }?
tanclary
reviewed
Feb 21, 2024
| @@ -245,6 +244,24 @@ private SqlValidatorNamespace resolveImpl(SqlIdentifier id) { | |||
| return rowType; | |||
| } | |||
|
|
|||
| @Override public void validateAlwaysFilter(Set<String> alwaysFilterFields) { | |||
| resolvedNamespace = resolveImpl(id); | |||
| if (resolvedNamespace instanceof TableNamespace) { | |||
There was a problem hiding this comment.
similar comment for some of these checks, we could change it to check if something is NOT true, and if it isn't then we just return, otherwise continue.
julianhyde
added a commit
to julianhyde/calcite
that referenced
this pull request
Feb 27, 2024
A table can declare that some of its columns must be filtered by implementing `interface SemanticTable`. If such columns are not filtered in a WHERE or HAVING clause, the validator throws. There are several purposes of these columns, one of which is to prevent expensive full-table scans (for example, reading all Orders without restricting on orderDate). Implementation is via the method SqlValidatorNamespace.getMustFilterFields(). For a table namespace, that method returns the declared must-filter fields. For a query namespce, that method returns any must-filter fields that have not been filtered in that query; such fields become the responsibility of the enclosing query. If a field is must-filter and is not in the SELECT clause of a sub-query, that is also an error, because of course it is now impossible for the enclosing query to filter it. Close apache#3688 Co-authored-by: Julian Hyde <jhyde@apache.org>
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.
No description provided.