Wrap expressions with parenthesis based on their precedence

Change-type: patch
See: https://balena.zulipchat.com/#narrow/stream/346007-balena-io.2FbalenaCloud/topic/pinejs.2015

Author: thgreasi

src/AbstractSQLRules2SQL.ts

@@ -387,14 +387,41 @@ const mathOps = {
 };
 export type MathOps = keyof typeof mathOps;
 
-const mathOperatorNodeTypes = new Set([
-	...Object.keys(mathOps),
-	'AddDateDuration',
-	'AddDateNumber',
-	'SubtractDateDate',
-	'SubtractDateDuration',
-	'SubtractDateNumber',
-]);
+const opsPrecedence = (() => {
+	const operatorsByPrecedence = [
+		['Multiply', 'Divide'],
+		[
+			'Add',
+			'AddDateDuration',
+			'AddDateNumber',
+			'Subtract',
+			'SubtractDateDate',
+			'SubtractDateDuration',
+			'SubtractDateNumber',
+		],
+		['Between', 'Like'],
+		[
+			'Equals',
+			'NotEquals',
+			'GreaterThan',
+			'GreaterThanOrEqual',
+			'LessThan',
+			'LessThanOrEqual',
+		],
+		// In, Exists, NotExists, 'IsDistinctFrom', 'IsNotDistinctFrom', Not,
+		// And, Or are already adding parenthesis.
+	] as const;
+
+	const operatorPrecedence = {} as Record<string, number>;
+	let precedence = 0;
+	for (const samePrecedenceOps of operatorsByPrecedence) {
+		for (const op of samePrecedenceOps) {
+			operatorPrecedence[op] = precedence;
+		}
+		precedence++;
+	}
+	return operatorPrecedence;
+})();
 
 const precedenceSafeOpValue = (
 	parentNodeType: string,
@@ -406,11 +433,12 @@ const precedenceSafeOpValue = (
 	const operandAbstractSql = getAbstractSqlQuery(args, index);
 	const valueExpr = valueMatchFn(operandAbstractSql, indent);
 	const [childNodeType] = operandAbstractSql;
+	const parentOperatorPrecedence = opsPrecedence[parentNodeType];
+	const childOperatorPrecedence = opsPrecedence[childNodeType];
 	if (
-		(mathOperatorNodeTypes.has(parentNodeType) &&
-			mathOperatorNodeTypes.has(childNodeType)) ||
-		// We need parenthesis for chained boolean comparisons, otherwise PostgreSQL complains.
-		(parentNodeType in comparisons && childNodeType in comparisons)
+		childOperatorPrecedence != null &&
+		parentOperatorPrecedence != null &&
+		parentOperatorPrecedence <= childOperatorPrecedence
 	) {
 		return `(${valueExpr})`;
 	}
@@ -1016,9 +1044,9 @@ const typeRules: Dictionary<MatchFn> = {
 	},
 	Between: (args, indent) => {
 		checkArgs('Between', args, 3);
-		const v = AnyValue(getAbstractSqlQuery(args, 0), indent);
-		const a = AnyValue(getAbstractSqlQuery(args, 1), indent);
-		const b = AnyValue(getAbstractSqlQuery(args, 2), indent);
+		const v = precedenceSafeOpValue('Between', AnyValue, args, 0, indent);
+		const a = precedenceSafeOpValue('Between', AnyValue, args, 1, indent);
+		const b = precedenceSafeOpValue('Between', AnyValue, args, 2, indent);
 		return `${v} BETWEEN ${a} AND (${b})`;
 	},
 	Add: MathOp('Add'),

test/abstract-sql/comparisons.ts

@@ -96,8 +96,9 @@ describe('Comparison Operator Precedence', () => {
 			it('should produce a valid NotEquals statement when the operands are math expressions with nested parenthesis', () => {
 				sqlEquals(
 					result.query,
+					// ideally we shouldn't have any parenthesis here
 					stripIndent`
-						SELECT 1 + (2 + (3 * 4)) != 1 + 0
+						SELECT 1 + (2 + 3 * 4) != 1 + 0
 					`,
 				);
 			});
@@ -159,7 +160,7 @@ describe('Comparison Operator Precedence', () => {
 				sqlEquals(
 					result.query,
 					stripIndent`
-						SELECT 1 + (2 * 5) >= 12 + 2
+						SELECT 1 + 2 * 5 >= 12 + 2
 					`,
 				);
 			});
@@ -330,6 +331,33 @@ describe('Comparison Operator Precedence', () => {
 		},
 	);
 
+	test(
+		[
+			'SelectQuery',
+			[
+				'Select',
+				[
+					[
+						'Between',
+						['Equals', ['Integer', 1], ['Integer', 0]],
+						['LessThan', ['Integer', 1], ['Integer', 0]],
+						['GreaterThan', ['Integer', 1], ['Integer', 0]],
+					],
+				],
+			],
+		],
+		(result, sqlEquals) => {
+			it('should produce a valid Between statement when the operands are comparison expressions', () => {
+				sqlEquals(
+					result.query,
+					stripIndent`
+						SELECT (1 = 0) BETWEEN (1 < 0) AND ((1 > 0))
+					`,
+				);
+			});
+		},
+	);
+
 	test(
 		[
 			'SelectQuery',
@@ -356,4 +384,31 @@ describe('Comparison Operator Precedence', () => {
 			});
 		},
 	);
+
+	test(
+		[
+			'SelectQuery',
+			[
+				'Select',
+				[
+					[
+						'Between',
+						['Equals', ['Integer', 1], ['Integer', 0]],
+						['LessThan', ['Integer', 1], ['Integer', 0]],
+						['GreaterThan', ['Integer', 1], ['Integer', 0]],
+					],
+				],
+			],
+		],
+		(result, sqlEquals) => {
+			it('should produce a valid Between statement when the operands are comparison expressions', () => {
+				sqlEquals(
+					result.query,
+					stripIndent`
+						SELECT (1 = 0) BETWEEN (1 < 0) AND ((1 > 0))
+					`,
+				);
+			});
+		},
+	);
 });

test/abstract-sql/math.ts

@@ -128,7 +128,7 @@ describe('Math Operator Precedence', () => {
 		],
 		(result, sqlEquals) => {
 			it('should produce a valid Add statement when the first operand is a Multiply', () => {
-				sqlEquals(result.query, 'SELECT (2 * 3) + 4');
+				sqlEquals(result.query, 'SELECT 2 * 3 + 4');
 			});
 		},
 	);
@@ -143,7 +143,7 @@ describe('Math Operator Precedence', () => {
 		],
 		(result, sqlEquals) => {
 			it('should produce a valid Add statement when the second operand is a Multiply', () => {
-				sqlEquals(result.query, 'SELECT 2 + (3 * 4)');
+				sqlEquals(result.query, 'SELECT 2 + 3 * 4');
 			});
 		},
 	);
@@ -164,7 +164,7 @@ describe('Math Operator Precedence', () => {
 		],
 		(result, sqlEquals) => {
 			it('should produce a valid Add statement of two Multiplications', () => {
-				sqlEquals(result.query, 'SELECT (2 * 3) + (4 * 5)');
+				sqlEquals(result.query, 'SELECT 2 * 3 + 4 * 5');
 			});
 		},
 	);
@@ -236,7 +236,7 @@ describe('Math Operator Precedence', () => {
 		],
 		(result, sqlEquals) => {
 			it('should produce a valid Subtract statement of two Multiplications', () => {
-				sqlEquals(result.query, 'SELECT (2 * 3) - (4 * 5)');
+				sqlEquals(result.query, 'SELECT 2 * 3 - 4 * 5');
 			});
 		},
 	);
@@ -257,7 +257,7 @@ describe('Math Operator Precedence', () => {
 		],
 		(result, sqlEquals) => {
 			it('should produce a valid Subtract statement of two Divisions', () => {
-				sqlEquals(result.query, 'SELECT (2 / 3) - (4 / 5)');
+				sqlEquals(result.query, 'SELECT 2 / 3 - 4 / 5');
 			});
 		},
 	);