Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue of filtering blobs with range conditions on tags #2537

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Blob:
- Fixed issue of download 0 size blob with range > 0 should have header "Content-Range: bytes \*/0" in returned error. (issue #2458)
- Aligned behavior with Azure to ignore invalid range requests for blob downloads. (issue #2458)
- Consider both Content-MD5 and x-ms-blob-content-md5 when creating a blob.
- Fixed issue of filtering blobs with correct multiple conditions on single tag (range queries). (issue #2514)

Table:

Expand Down
84 changes: 42 additions & 42 deletions src/blob/persistence/QueryInterpreter/QueryParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ export default function parseQuery(

/**
* A recursive descent parser for Azure Blob filter by tags query syntax.
*
*
* This parser is implemented using a recursive descent strategy, which composes
* layers of syntax hierarchy, roughly corresponding to the structure of an EBNF
* grammar. Each layer of the hierarchy is implemented as a method which consumes
* the syntax for that layer, and then calls the next layer of the hierarchy.
*
*
* So for example, the syntax tree that we currently use is composed of:
* - QUERY := EXPRESSION
* - EXPRESSION := OR
Expand Down Expand Up @@ -89,13 +89,13 @@ class QueryParser {
}

if (currentComparison === ComparisonType.Greater &&
(this.comparisonNodes[key].existedComparison[i] === ComparisonType.Less
(this.comparisonNodes[key].existedComparison[i] === ComparisonType.Greater
|| this.comparisonNodes[key].existedComparison[i] === ComparisonType.Equal)) {
throw new Error("can't have multiple conditions for a single tag unless they define a range");
}

if (currentComparison === ComparisonType.Less &&
(this.comparisonNodes[key].existedComparison[i] === ComparisonType.Greater
(this.comparisonNodes[key].existedComparison[i] === ComparisonType.Less
|| this.comparisonNodes[key].existedComparison[i] === ComparisonType.Equal)) {
throw new Error("can't have multiple conditions for a single tag unless they define a range");
}
Expand All @@ -105,7 +105,7 @@ class QueryParser {
return;
}

appendComparionNode(key: string, currentComparison: ComparisonType) {
appendComparisonNode(key: string, currentComparison: ComparisonType) {
if (this.conditionHeader) {
return;
}
Expand Down Expand Up @@ -141,7 +141,7 @@ class QueryParser {

/**
* Visits the root of the query syntax tree, returning the corresponding root node.
*
*
* @returns {IQueryNode}
*/
visit(): IQueryNode {
Expand All @@ -150,7 +150,7 @@ class QueryParser {

/**
* Visits the QUERY layer of the query syntax tree, returning the appropriate node.
*
*
* @returns {IQueryNode}
*/
private visitQuery(): IQueryNode {
Expand All @@ -164,9 +164,9 @@ class QueryParser {

/**
* Visits the EXPRESSION layer of the query syntax tree, returning the appropriate node.
*
*
* EXPRESSION := OR
*
*
* @returns {IQueryNode}
*/
private visitExpression(): IQueryNode {
Expand All @@ -175,9 +175,9 @@ class QueryParser {

/**
* Visits the OR layer of the query syntax tree, returning the appropriate node.
*
*
* OR := AND ("or" OR)*
*
*
* @returns {IQueryNode}
*/
private visitOr(): IQueryNode {
Expand All @@ -197,9 +197,9 @@ class QueryParser {

/**
* Visits the AND layer of the query syntax tree, returning the appropriate node.
*
*
* AND := UNARY ("and" AND)*
*
*
* @returns {IQueryNode}
*/
private visitAnd(): IQueryNode {
Expand All @@ -217,9 +217,9 @@ class QueryParser {

/**
* Visits the UNARY layer of the query syntax tree, returning the appropriate node.
*
*
* UNARY := ("not")? EXPRESSION_GROUP
*
*
* @returns {IQueryNode}
*/
private visitUnary(): IQueryNode {
Expand All @@ -230,9 +230,9 @@ class QueryParser {

/**
* Visits the EXPRESSION_GROUP layer of the query syntax tree, returning the appropriate node.
*
*
* EXPRESSION_GROUP := ("(" OR ")") | BINARY
*
*
* @returns {IQueryNode}
*/
private visitExpressionGroup(): IQueryNode {
Expand All @@ -251,9 +251,9 @@ class QueryParser {

/**
* Visits the BINARY layer of the query syntax tree, returning the appropriate node.
*
*
* BINARY := IDENTIFIER_OR_CONSTANT (OPERATOR IDENTIFIER_OR_CONSTANT)?
*
*
* @returns {IQueryNode}
*/
private visitBinary(): IQueryNode {
Expand All @@ -267,30 +267,30 @@ class QueryParser {
switch (operator) {
case "=":
this.validateWithPreviousComparison(left.toString(), ComparisonType.Equal);
this.appendComparionNode(left.toString(), ComparisonType.Equal);
this.appendComparisonNode(left.toString(), ComparisonType.Equal);
return new EqualsNode(left, right);
case "<>":
if (!this.conditionHeader) {
this.query.throw(`unexpected <>`);
}
this.validateWithPreviousComparison(left.toString(), ComparisonType.NotEqual);
this.appendComparionNode(left.toString(), ComparisonType.NotEqual);
this.appendComparisonNode(left.toString(), ComparisonType.NotEqual);
return new NotEqualsNode(left, right);
case ">=":
this.validateWithPreviousComparison(left.toString(), ComparisonType.Greater);
this.appendComparionNode(left.toString(), ComparisonType.Greater);
this.appendComparisonNode(left.toString(), ComparisonType.Greater);
return new GreaterThanEqualNode(left, right);
case ">":
this.validateWithPreviousComparison(left.toString(), ComparisonType.Greater);
this.appendComparionNode(left.toString(), ComparisonType.Greater);
this.appendComparisonNode(left.toString(), ComparisonType.Greater);
return new GreaterThanNode(left, right);
case "<":
this.validateWithPreviousComparison(left.toString(), ComparisonType.Less);
this.appendComparionNode(left.toString(), ComparisonType.Less);
this.appendComparisonNode(left.toString(), ComparisonType.Less);
return new LessThanNode(left, right);
case "<=":
this.validateWithPreviousComparison(left.toString(), ComparisonType.Less);
this.appendComparionNode(left.toString(), ComparisonType.Less);
this.appendComparisonNode(left.toString(), ComparisonType.Less);
return new LessThanEqualNode(left, right);
}
}
Expand All @@ -300,9 +300,9 @@ class QueryParser {

/**
* Visits the IDENTIFIER_OR_CONSTANT layer of the query syntax tree, returning the appropriate node.
*
*
* IDENTIFIER_OR_CONSTANT := CONSTANT | IDENTIFIER
*
*
* @returns {IQueryNode}
*/
private visitValue(): IQueryNode {
Expand Down Expand Up @@ -370,7 +370,7 @@ class QueryParser {

/**
* Visits the STRING layer of the query syntax tree, returning the appropriate node.
*
*
* Strings are wrapped in either single quotes (') or double quotes (") and may contain
* doubled-up quotes to introduce a literal.
*/
Expand All @@ -381,15 +381,15 @@ class QueryParser {
* Strings are terminated by the same character that opened them.
* But we also allow doubled-up characters to represent a literal, which means we need to only terminate a string
* when we receive an odd-number of closing characters followed by a non-closing character.
*
*
* Conceptually, this is represented by the following state machine:
*
*
* - start: normal
* - normal+(current: !') -> normal
* - normal+(current: ', next: ') -> escaping
* - normal+(current: ', next: !') -> end
* - escaping+(current: ') -> normal
*
*
* We can implement this using the state field of the `take` method's predicate.
*/
const content = this.query.take((c, peek, state) => {
Expand Down Expand Up @@ -420,9 +420,9 @@ class QueryParser {

/**
* Visits the IDENTIFIER layer of the query syntax tree, returning the appropriate node.
*
*
* Identifiers are a sequence of characters which are not whitespace.
*
*
* @returns {IQueryNode}
*/
private visitKey(): IQueryNode {
Expand Down Expand Up @@ -455,7 +455,7 @@ export class ParserContext {

/**
* Asserts that the query has been fully consumed.
*
*
* This method should be called after the parser has finished consuming the known parts of the query.
* Any remaining query after this point is indicative of a syntax error.
*/
Expand All @@ -467,7 +467,7 @@ export class ParserContext {

/**
* Retrieves the next character in the query without advancing the parser.
*
*
* @returns {string} A single character, or `undefined` if the end of the query has been reached.
*/
peek(): string {
Expand All @@ -486,7 +486,7 @@ export class ParserContext {
/**
* Attempts to consume a given sequence of characters from the query,
* advancing the parser if the sequence is found.
*
*
* @param {string} sequence The sequence of characters which should be consumed.
* @param {boolean} ignoreCase Whether or not the case of the characters should be ignored.
* @returns {boolean} `true` if the sequence was consumed, `false` otherwise.
Expand All @@ -505,11 +505,11 @@ export class ParserContext {
/**
* Attempts to consume one of a given set of sequences from the query,
* advancing the parser if one of the sequences is found.
*
*
* Sequences are tested in the order they are provided, and the first
* sequence which is found is consumed. As such, it is important to
* avoid prefixes appearing before their longer counterparts.
*
*
* @param {boolean} ignoreCase Whether or not the case of the characters should be ignored.
* @param {string[]} options The list of character sequences which should be consumed.
* @returns {string | null} The sequence which was consumed, or `null` if none of the sequences were found.
Expand All @@ -526,10 +526,10 @@ export class ParserContext {

/**
* Consumes a sequence of characters from the query based on a character predicate function.
*
*
* The predicate function is called for each character in the query, and the sequence is
* consumed until the predicate returns `false` or the end of the query is reached.
*
*
* @param {Function} predicate The function which determines which characters should be consumed.
* @returns {string} The sequence of characters which were consumed.
*/
Expand Down Expand Up @@ -559,10 +559,10 @@ export class ParserContext {
/**
* Consumes a sequence of characters from the query based on a character predicate function,
* and then consumes a terminating sequence of characters (throwing an exception if these are not found).
*
*
* This function is particularly useful for consuming sequences of characters which are surrounded
* by a prefix and suffix, such as strings.
*
*
* @param {string} prefix The prefix which should be consumed.
* @param {Function} predicate The function which determines which characters should be consumed.
* @param {string} suffix The suffix which should be consumed.
Expand Down
Loading
Loading