Support for Unnest on Top-Level Array Fields in Flat Postgres Collection #239

suddendust · 2025-10-06T07:28:49Z

Description

This PR adds support for un-nesting top-level array fields in flat PG collection. Currently, it assumes that the column defined in UnnestExpression is an array. Un-nesting JSON arrays in currently not support in flat collections.

Testing

Have added DocStoreQueryV1Test#testFlatPostgresCollectionUnnestTags to test the behaviour.

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

This reverts commit 674d2ee.

This reverts commit f6cb67a.

This reverts commit 6e7795d.

This reverts commit 957a897.

… into pg_subdocument

…ctor

codecov · 2025-10-06T07:30:22Z

Codecov Report

❌ Patch coverage is 83.54430% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.87%. Comparing base (111e083) to head (e209d88).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...1/vistors/PostgresFilterTypeExpressionVisitor.java	67.50%	4 Missing and 9 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #239      +/-   ##
============================================
+ Coverage     79.73%   79.87%   +0.13%     
  Complexity     1099     1099              
============================================
  Files           213      213              
  Lines          5344     5396      +52     
  Branches        455      473      +18     
============================================
+ Hits           4261     4310      +49     
+ Misses          763      760       -3     
- Partials        320      326       +6

Flag	Coverage Δ
integration	`79.87% <83.54%> (+0.13%)`	⬆️
unit	`56.89% <26.58%> (-0.45%)`	⬇️

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 · 2025-10-08T08:08:27Z

...nt-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java

+      // From looking at the data:
+      // - "hygiene" appears in docs 1, 5, 8 = 3 times
+      // - "personal-care" appears in docs 1, 3 = 2 times
+      // - "grooming" appears in docs 6, 7 = 2 times


This is nice. Thanks a lot for these inline comments. 🙂

suresh-prakash · 2025-10-08T08:11:48Z

...rtrace/core/documentstore/postgres/query/v1/vistors/PostgresFilterTypeExpressionVisitor.java

-    return String.format(
-        "EXISTS (SELECT 1 FROM jsonb_array_elements(COALESCE(%s, '[]'::jsonb)) AS \"%s\" WHERE %s)",
-        parsedLhs, alias, parsedFilter);
+    if (isFlatCollection) {


Apologies if this comment is coming pretty late.
The more and more of these conditions make me feel that it would be better to separate the "FlatCollection Visitors" and "NestedCollection Visitors" in different classes backed by some factory, avoiding a lot of these checks. More than the checks, it'll be much cleaner to maintain/debug.

Having said that, I leave it upto you if the distinction really makes sense.

Sure, this makes sense. Let me see how much of this can be refactored. Thanks!

Apologies if this comment is coming pretty late. The more and more of these conditions make me feel that it would be better to separate the "FlatCollection Visitors" and "NestedCollection Visitors" in different classes backed by some factory, avoiding a lot of these checks. More than the checks, it'll be much cleaner to maintain/debug.

Having said that, I leave it upto you if the distinction really makes sense.

+1

@puneet-traceable @suresh-prakash I did a bit of refactoring and the changes are substantial. How about we merge this PR and take that up in a separate one? To keep things cleaner.

Sure. That works.

suresh-prakash · 2025-10-08T08:12:44Z

.../core/documentstore/postgres/query/v1/vistors/PostgresUnnestFilterTypeExpressionVisitor.java

+              postgresQueryParser.getQuery().getFilter(), postgresQueryParser);
+
+      if (StringUtils.isNotEmpty(unnestFilters) && mainFilter.isPresent()) {
+        return Optional.of(unnestFilters + " AND " + mainFilter.get());


Nit: Prefer StringBuilder, StringBuffer or String.format()

This reverts commit 36f2892.

suddendust · 2025-10-13T06:32:20Z

@suresh-prakash @puneet-traceable For queries like SELECT item, price FROM <implicit_collection> WHERE ANY(numbers) = 10, the parser tries to cast the array into the apt type (SELECT 1 FROM unnest(COALESCE(%s, ARRAY[]%s) in PostgresFilterTypeExpressionVisitor). This type inference fails when:

Filter is a LogicalExpression (AND/OR) rather than RelationalExpression.
Filter compares against an IdentifierExpression instead of a ConstantExpression.

We need a better way to do the type cast, but don't really see any other way. Your thoughts?

suresh-prakash · 2025-10-16T05:05:33Z

@suresh-prakash @puneet-traceable For queries like SELECT item, price FROM <implicit_collection> WHERE ANY(numbers) = 10, the parser tries to cast the array into the apt type (SELECT 1 FROM unnest(COALESCE(%s, ARRAY[]%s) in PostgresFilterTypeExpressionVisitor). This type inference fails when:

Filter is a LogicalExpression (AND/OR) rather than RelationalExpression.

Filter compares against an IdentifierExpression instead of a ConstantExpression.

We need a better way to do the type cast, but don't really see any other way. Your thoughts?

Filter is a LogicalExpression (AND/OR) rather than RelationalExpression.

I guess, we probably hadn't seen such use-cases yet. Wondering if it would make sense to break the logical expression into component relational expressions before inferring type for each composed filter. 🤔

Filter compares against an IdentifierExpression instead of a ConstantExpression.

Are there any use cases for this?

suddendust and others added 30 commits September 22, 2025 15:04

WIP

674d2ee

Revert "WIP"

f6cb67a

This reverts commit 674d2ee.

Reapply "WIP"

6e7795d

This reverts commit f6cb67a.

Revert "Reapply "WIP""

957a897

This reverts commit 6e7795d.

Reapply "Reapply "WIP""

502c373

This reverts commit 957a897.

WIP

0544358

WIP

82dc37f

Add default impl for getDocumentType

c0ed72a

WIP

e9f25da

Merge branch 'pg_subdocument' of github.com:suddendust/document-store…

e6be8b9

… into pg_subdocument

Rollback unnecessary changes

ee7678f

Added default DocumentType in Document.java

63b0dec

Address comments

7f91cf0

Merge branch 'main' into pg_subdocument

7376b56

Address comments

ea2aefd

Merge remote-tracking branch 'myfork/pg_subdocument' into pg_subdocument

e3ccb5b

Address docs

3e3e6cf

Adds more test cases in JSONDocumentTest.java

e6a6f7e

Spotless

483eb16

Added more test cases

e5926ea

Spotless

b261463

Add test case for PostgresResultIteratorWithBasicTypes

7cf8e17

Merge branch 'main' into pg_subdocument

9c474db

WIP

cb2ed18

Merge branch 'pg_subdocument' into poc

06fdc8f

WIP

fbce388

Merge branch 'main' of github.com:hypertrace/document-store into refa…

50b09b7

…ctor

WIP

0f8df71

WIP

7f7defe

Added test cases

13e591e

WIP

5eda1b8

suddendust requested review from avinashkolluru, kotharironak, skjindal93 and suresh-prakash as code owners October 6, 2025 07:28

suddendust added 4 commits October 6, 2025 13:11

WIP

4f8aafd

WIP

702b30e

WIP

7a6b799

WIP

a42d48b

suddendust changed the title ~~[Draft] Support for Unnest on Top-Level Array Fields in Flat Postgres Collection~~ Support for Unnest on Top-Level Array Fields in Flat Postgres Collection Oct 8, 2025

Added test docs

2d016a3

suresh-prakash previously approved these changes Oct 8, 2025

View reviewed changes

suddendust added 3 commits October 8, 2025 15:41

Separate flat and nested visitors

36f2892

Revert "Separate flat and nested visitors"

d4afa05

This reverts commit 36f2892.

Fix compilation issue

b10a9a6

suddendust dismissed suresh-prakash’s stale review via b10a9a6 October 12, 2025 17:51

suddendust added 3 commits October 13, 2025 01:58

Added more test cases

d28bd87

Fix failing test case

8279987

Test case to cover missed lines

16172f4

suddendust requested a review from suresh-prakash October 13, 2025 05:49

suddendust added 2 commits October 13, 2025 11:37

Test case to cover missed lines

93af3f1

Added additional test cases

b1a75c1

suddendust and others added 2 commits October 13, 2025 12:46

Add todo

92742b3

Merge branch 'main' into flat_collection_unnest

1a0c33f

suresh-prakash approved these changes Oct 14, 2025

View reviewed changes

Merge branch 'main' into flat_collection_unnest

e209d88

suresh-prakash merged commit 5c65ef6 into hypertrace:main Oct 15, 2025
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support for Unnest on Top-Level Array Fields in Flat Postgres Collection #239

Support for Unnest on Top-Level Array Fields in Flat Postgres Collection #239

Uh oh!

suddendust commented Oct 6, 2025 •

edited

Loading

Uh oh!

codecov bot commented Oct 6, 2025 •

edited

Loading

Uh oh!

suresh-prakash Oct 8, 2025

Uh oh!

suresh-prakash Oct 8, 2025

Uh oh!

suddendust Oct 8, 2025

Uh oh!

puneet-traceable Oct 8, 2025

Uh oh!

suddendust Oct 9, 2025

Uh oh!

suresh-prakash Oct 9, 2025

Uh oh!

suresh-prakash Oct 8, 2025

Uh oh!

suddendust commented Oct 13, 2025 •

edited

Loading

Uh oh!

Uh oh!

suresh-prakash commented Oct 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Support for Unnest on Top-Level Array Fields in Flat Postgres Collection #239

Support for Unnest on Top-Level Array Fields in Flat Postgres Collection #239

Uh oh!

Conversation

suddendust commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Checklist:

Uh oh!

codecov bot commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

suresh-prakash Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

suresh-prakash Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

suddendust Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

puneet-traceable Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

suddendust Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

suresh-prakash Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

suresh-prakash Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

suddendust commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

suresh-prakash commented Oct 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

suddendust commented Oct 6, 2025 •

edited

Loading

codecov bot commented Oct 6, 2025 •

edited

Loading

suddendust commented Oct 13, 2025 •

edited

Loading