fix: span a binary condition's node from its left operand

[knutwannheden]

A node built for a binary condition does not cover the operand the condition is about. Nested in an AND, so that the condition is not also the outermost expression:

String sql = "select x from t where z = 0 and a in (1, 2)";
Expression condition = ((AndExpression) ((PlainSelect) CCJSqlParserUtil.parse(sql)).getWhere())
        .getRightExpression();
Node node = ((ASTNodeAccess) condition).getASTNode();
sql.substring(node.jjtGetFirstToken().absoluteBegin - 1,
              node.jjtGetLastToken().absoluteEnd - 1);   // "in (1, 2)", not "a in (1, 2)"

Seven productions take their left operand as a parameter — RegularConditionRHS, InExpression, IncludesExpression, ExcludesExpression, LikeExpression, SimilarToExpression, IsDistinctExpression. The caller has consumed those tokens by the time JJTree opens the node, so its range starts at the operator.

Which half is a regression

nested in AND	5.3	master	this PR
a = 1	a = 1	= 1	a = 1
a in (1, 2)	in (1, 2)	in (1, 2)	a in (1, 2)
a like 'p'	like 'p'	like 'p'	a like 'p'
a similar to 'p'	similar to 'p'	similar to 'p'	a similar to 'p'
a is distinct from 1	is distinct from 1	is distinct from 1	a is distinct from 1

Only the comparison row is new. 1b51f05 split RegularCondition() into RegularConditionRHS(Expression leftExpression, int) for the Pratt rewrite, which moved comparisons onto the same footing as the rest of the family.

Worth saying plainly: 5.3 was not consistent here either. Comparisons spanned their operand and the other four did not, so the refactor did not introduce the defect so much as finish spreading it. Fixing only the comparison row would restore 5.3 while leaving a = 1 and a in (1, 2) disagreeing inside a single WHERE, so this fixes all seven.

A condition is only affected while it is not the outermost expression of its clause — at the top of a WHERE the surrounding production has already opened the node, and the range comes out right. That is why SELECT * FROM t WHERE a IN (1,2) looks correct and the same condition behind an AND does not.

The behaviour change

SelectASTTest.testDetectInExpressions pinned the narrow range and moves with this:

-        assertEquals(32, subSelectStart.beginColumn);
+        // the node spans the whole IN expression, so it starts at the left operand
+        assertEquals(30, subSelectStart.beginColumn);

Column 32 is the IN keyword, column 30 the a it tests. I read the old value as describing what the parser did rather than what it owed the caller, but it is a visible change for anyone reading jjtGetFirstToken() off these nodes, and it is the reason this PR is worth a second look rather than a rubber stamp. Happy to narrow it to the comparison regression alone if you would rather keep the existing IN range.

Not addressed

Between, IsNullExpression, IsBooleanExpression, IsUnknownExpression, MemberOfExpression and OverlapsCondition take a left operand on the same pattern but never call linkAST, so getASTNode() returns null for them in 5.3 and on master alike. Separate gap, left alone.

Testing

testBinaryConditionNodeStartsAtItsLeftOperand walks one nested case per fixed production and asserts each node maps back to its own source text. Full suite: 6607 tests, 0 failures.

[manticore-projects]

Good job! Thank you for correcting this, appreciated!