Skip to content

Conversation

@suddendust
Copy link
Contributor

@suddendust suddendust commented Nov 25, 2025

Description

This PR fixes critical bugs in Postgres path.

1. Unquoted PG Col Names Containing Hyphens

We were not quoting PG col names if it was a JSON array (bug). Due to this, these fields were being referred to as p1(customAttribute_dot_dev-ops-owner) for example. PG throws an unknown character exception for this. The correct way to do this is to quote the col name as p1("customAttribute_dot_dev-ops-owner").

2. PreparedStatement Misinterpreting the ? Operator

For JSONB scalar fields, we were using the parent ? child expr. ? is the contains operator. But prepared statement incorrectly interprets it as a placeholder and fills in a value. For now, I have reverted it to using IS NULL/IS NOT NULL.

3. Handle Unnest + EXISTS/NOT_EXISTS on Native/JSONB Arrays Correctly

For queries like:

          Query.builder()
              .addSelection(IdentifierExpression.of("item"))
              .addSelection(ArrayIdentifierExpression.of("tags"))
              .addFromClause(
                  UnnestExpression.of(ArrayIdentifierExpression.of("tags", ArrayType.TEXT), true))
              .setFilter(
                  RelationalExpression.of(
                      ArrayIdentifierExpression.of("tags", ArrayType.TEXT),
                      NOT_EXISTS,
                      ConstantExpression.of("null")))
              .build();

In which we unnest an array and then apply a filter EMPTY/IS_NOT_EMPTY on the array col, the parser was generating an incorrect query. This has been fixed (please see the UTs for the correct behaviour).

4. Handle Unnest + IN/NOT_IN on Native/JSONB Arrays Correctly

This is similar to the previous case, for queries in which we first unnest an array col, and then apply a filter on it.

5. Handle CONTAINS/NOT_CONTAINS on Top Level Array Fields

This wasn't working till now.

Testing

  1. Added UTs and integration tests.
  2. Tested these changes in a live environment.

Checklist:

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

❌ Patch coverage is 83.05085% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.39%. Comparing base (6526d3c) to head (18bf317).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ter/PostgresInRelationalFilterParserJsonArray.java 60.00% 5 Missing and 3 partials ⚠️
...ore/documentstore/postgres/PostgresCollection.java 81.81% 4 Missing and 2 partials ⚠️
...resContainsRelationalFilterParserNonJsonField.java 71.42% 2 Missing and 4 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #253      +/-   ##
============================================
+ Coverage     80.02%   80.39%   +0.36%     
- Complexity     1249     1256       +7     
============================================
  Files           227      227              
  Lines          5756     5834      +78     
  Branches        500      517      +17     
============================================
+ Hits           4606     4690      +84     
+ Misses          805      790      -15     
- Partials        345      354       +9     
Flag Coverage Δ
integration 80.39% <83.05%> (+0.36%) ⬆️
unit 57.64% <29.66%> (-0.42%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@suresh-prakash suresh-prakash merged commit 1e7c791 into hypertrace:main Nov 26, 2025
6 checks passed
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