Skip to content

Conversation

@JohnFitzpatrick44
Copy link
Member

Description

Fixes #19695 . Addresses issues with round-trips for negated subqueries. Specifically, the formatter adds parentheses around negative unary arithmetic expressions, even if the value being negated already has parentheses around it (i.e. for SQL subqueries).

This change just checks whether the value being negated is already wrapped in parentheses.

Additional context and related issues

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Dec 17, 2025
@ebyhr
Copy link
Member

ebyhr commented Dec 17, 2025

Could you please add a test?

@JohnFitzpatrick44
Copy link
Member Author

@ebyhr I meant to ask where a good place to add the test would be. I know Trino typically leans hard on integration tests, but this seems much better suited for a unit test.

I see that there's a TestExpressionFormatter test, but I've tested this change locally by creating a new TestSqlFormatter test by hitting the SqlFormatterUtil.getFormattedSql method to confirm a query round-trips. Do you have a preference?

@JohnFitzpatrick44 JohnFitzpatrick44 force-pushed the jcf/fix-formatting-on-negation branch 2 times, most recently from ffef46f to 18ab834 Compare December 23, 2025 18:40
private static Stream<String> sqlQueries()
{
return Stream.of(
"SELECT +1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious if the following would be a valid case:
(SELECT 1) + SELECT
(SELECT 1) - SELECT

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SqlFormatter seems to correctly throw a syntax error for these cases

@JohnFitzpatrick44 JohnFitzpatrick44 force-pushed the jcf/fix-formatting-on-negation branch from 18ab834 to 1468546 Compare January 26, 2026 17:11
@JohnFitzpatrick44 JohnFitzpatrick44 force-pushed the jcf/fix-formatting-on-negation branch 3 times, most recently from 082289b to aa33639 Compare January 26, 2026 21:39
@wendigo wendigo force-pushed the jcf/fix-formatting-on-negation branch from aa33639 to 1a3faec Compare January 28, 2026 10:52
@wendigo wendigo merged commit b487944 into trinodb:master Jan 28, 2026
4 of 13 checks passed
@github-actions github-actions bot added this to the 480 milestone Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Trino Formatting Issue with Negation in SQL Queries

4 participants