Skip to content

Commit

Permalink
Fix parsing && and || operators
Browse files Browse the repository at this point in the history
  • Loading branch information
MikeShi42 committed Jan 18, 2024
1 parent 5f040b3 commit c9e51ac
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 4 deletions.
6 changes: 6 additions & 0 deletions .changeset/moody-falcons-know.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@hyperdx/api': patch
'@hyperdx/app': patch
---

fix: Fixed parsing && and || operators in queries correctly
31 changes: 31 additions & 0 deletions packages/api/src/clickhouse/__tests__/searchQueryParser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,37 @@ describe('searchQueryParser', () => {
});
});

describe('operators', () => {
(
[
['OR', 'OR'],
['||', 'OR'],
['AND', 'AND'],
['&&', 'AND'],
[' ', 'AND'],
['NOT', 'AND NOT'],
['AND NOT', 'AND NOT'],
['OR NOT', 'OR NOT'],
] as const
).forEach(([operator, sql]) => {
it(`parses ${operator}`, async () => {
const ast = parse(`foo ${operator} bar`);
expect(
await genWhereSQL(
ast,
propertyTypesMappingsModel,
'TEAM_ID_UNIT_TESTS',
),
).toEqual(
`${hasToken(SOURCE_COL, 'foo')} ${sql} ${hasToken(
SOURCE_COL,
'bar',
)}`,
);
});
});
});

describe('properties', () => {
it('parses string property values', async () => {
const ast = parse('foo:bar');
Expand Down
25 changes: 23 additions & 2 deletions packages/api/src/clickhouse/searchQueryParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ export const buildSearchColumnName = (
};

interface Serializer {
operator(op: lucene.Operator): string;
eq(field: string, term: string, isNegatedField: boolean): Promise<string>;
isNotNull(field: string, isNegatedField: boolean): Promise<string>;
gte(field: string, term: string): Promise<string>;
Expand Down Expand Up @@ -189,6 +190,27 @@ export class SQLSerializer implements Serializer {
};
}

operator(op: lucene.Operator) {
switch (op) {
case 'NOT':
case 'AND NOT':
return 'AND NOT';
case 'OR NOT':
return 'OR NOT';
// @ts-ignore TODO: Types need to be fixed upstream
case '&&':
case '<implicit>':
case 'AND':
return 'AND';
// @ts-ignore TODO: Types need to be fixed upstream
case '||':
case 'OR':
return 'OR';
default:
throw new Error(`Unexpected operator. ${op}`);
}
}

async eq(field: string, term: string, isNegatedField: boolean) {
const { column, found, propertyType } = await this.getColumnForField(field);
if (!found) {
Expand Down Expand Up @@ -485,8 +507,7 @@ async function serialize(
// 2. LeftOnlyAST: Single term ex. "foo:bar"
if ((ast as lucene.BinaryAST).right != null) {
const binaryAST = ast as lucene.BinaryAST;
const operator =
binaryAST.operator === IMPLICIT_FIELD ? 'AND' : binaryAST.operator;
const operator = serializer.operator(binaryAST.operator);
const parenthesized = binaryAST.parenthesized;
return `${parenthesized ? '(' : ''}${await serialize(
binaryAST.left,
Expand Down
25 changes: 23 additions & 2 deletions packages/app/src/queryv2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export function parse(query: string): lucene.AST {
const IMPLICIT_FIELD = '<implicit>';

interface Serializer {
operator(op: lucene.Operator): string;
eq(field: string, term: string, isNegatedField: boolean): Promise<string>;
isNotNull(field: string, isNegatedField: boolean): Promise<string>;
gte(field: string, term: string): Promise<string>;
Expand Down Expand Up @@ -56,6 +57,27 @@ class EnglishSerializer implements Serializer {
return `'${field}'`;
}

operator(op: lucene.Operator) {
switch (op) {
case 'NOT':
case 'AND NOT':
return 'AND NOT';
case 'OR NOT':
return 'OR NOT';
// @ts-ignore TODO: Types need to be fixed upstream
case '&&':
case '<implicit>':
case 'AND':
return 'AND';
// @ts-ignore TODO: Types need to be fixed upstream
case '||':
case 'OR':
return 'OR';
default:
throw new Error(`Unexpected operator. ${op}`);
}
}

async eq(field: string, term: string, isNegatedField: boolean) {
return `${this.translateField(field)} ${
isNegatedField ? 'is not' : 'is'
Expand Down Expand Up @@ -284,8 +306,7 @@ async function serialize(
// 2. LeftOnlyAST: Single term ex. "foo:bar"
if ((ast as lucene.BinaryAST).right != null) {
const binaryAST = ast as lucene.BinaryAST;
const operator =
binaryAST.operator === '<implicit>' ? 'AND' : binaryAST.operator;
const operator = serializer.operator(binaryAST.operator);
const parenthesized = binaryAST.parenthesized;
return `${parenthesized ? '(' : ''}${await serialize(
binaryAST.left,
Expand Down

0 comments on commit c9e51ac

Please sign in to comment.