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

Improving parenthesizing of binary expressions #1258

Merged
merged 11 commits into from
Dec 25, 2023

Conversation

trueqbit
Copy link
Collaborator

The serialization of binary expressions currently suffers from two problems:

  1. The statement serializer too eagerly puts parentheses around every built-in function and binary expression (comparison, logic, arithmetic, bitwise operators) and is additionally dependent on a flag provided by the outer context.
  2. Subqueries in binary expressions in columns currently only work because the outer expressions do not change the serializer_context<>::use_parentheses=true flag. As soon as it is changed, as in CTEs, the serialization goes wrong.
  1. can be improved and 2) remedied in the following ways:
  • Binary operators/conditions: Parentheses are only needed to preserve precedence by following simple logic: only "subtree" expressions need to be parenthesized, "leaf" expressions do not.
  • No parentheses around built-in functions necessary - they are already independent expressions (and the outer expression already takes over the parentheses, e.g. check(), default()).
  • Establishing a parenthesized subcontext for potential subqueries in binary expressions, such that select(&Employee::salary >= select(avg(&Employee::salary))) works.

This leads to a simplified approach, as the serializer is independent of the context.
In addition, fewer characters are serialized with simple binary expressions, which also improves the readability of statements when they are examined.

Also in this PR:

  • Allow conditional expressions to be equality-comparable, so expressions like select(c(5) > 3 == true) are possible instead of select(c(5) > 3 == c(true))
  • Merged handling of binary conditions and operators (AST iteration, serialization)
  • Appveyor vcpkg environmnent updated to 2023.12.12.
  • Fixed "left_and_inner_join" example

examples/left_and_inner_join.cpp Outdated Show resolved Hide resolved
examples/subquery.cpp Show resolved Hide resolved
@fnc12
Copy link
Owner

fnc12 commented Dec 24, 2023

@trueqbit everything is brilliant (as always) except two items

@trueqbit trueqbit merged commit a7fa2d2 into fnc12:dev Dec 25, 2023
2 checks passed
@trueqbit trueqbit deleted the binary_condition_parentheses branch December 25, 2023 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants