Skip to content

Conversation

@suddendust
Copy link
Contributor

@suddendust suddendust commented Nov 19, 2025

Description

There are certain cases in which we want to differentiate primitive from array fields in flat collections. For example, if we want to add a filter to an array column that cardinality(col) > 0. This information can also be used to apply array/primitive level optimisations while generating the PG queries.

For example, consider this query: SELECT "id" AS "API.id", "attributes.name" AS "API.name" FROM "entities_api" WHERE (ARRAY["id"]::text[] && ARRAY['0437f39b-19c7-3cdc-a91b-2304b15e85d7', '05162aaf-a0b6-3e76-a8a2-f82fda639d4d', '05e318bc-eda0-395a-8dc5-a9a642858911']::text[]) AND ("tenantId" = 'df6f1cc2-9f7f-4633-bf0c-ddb1c510dcde') OFFSET 0 LIMIT 10

Here, id is a primitive but even then we're casting it as an array and using the overlap operator to check if it's contained in the RHS array. This is very inefficient. With this type info, we'd be able to use primitive operators for such cases.

Screenshot 2025-11-19 at 11 44 11 AM

Testing

Currently, these classes are not used anywhere. Tests would be added as-and-when they start getting used.

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 19, 2025

Codecov Report

❌ Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.49%. Comparing base (23c12e2) to head (c07ee86).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...expression/impl/JsonArrayIdentifierExpression.java 84.61% 0 Missing and 2 partials ⚠️
...ore/expression/impl/ArrayIdentifierExpression.java 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main     #249   +/-   ##
=========================================
  Coverage     80.48%   80.49%           
- Complexity     1151     1162   +11     
=========================================
  Files           215      217    +2     
  Lines          5533     5551   +18     
  Branches        489      490    +1     
=========================================
+ Hits           4453     4468   +15     
  Misses          753      753           
- Partials        327      330    +3     
Flag Coverage Δ
integration 80.49% <83.33%> (+<0.01%) ⬆️
unit 57.68% <83.33%> (+0.08%) ⬆️

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.

@puneet-traceable
Copy link

Looks good to me.
But one thing worth considering is:
Should we go this route or we introduce type information in IdentifierExpression directly which gives us flexibility to cater to specific needs per type if it arises.

suresh-prakash
suresh-prakash previously approved these changes Nov 19, 2025
@suddendust
Copy link
Contributor Author

Should we go this route or we introduce type information in IdentifierExpression directly which gives us flexibility to cater to specific needs per type if it arises.

Indeed. If we're creating too many classes (one for each type), we can switch to this approach. Sticking to this now since such a need hasn't arisen yet. @suresh-prakash

@puneet-traceable puneet-traceable merged commit 61e844b into hypertrace:main Nov 19, 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.

3 participants