Support for SELECT on nested JSON fields in Flat Collections #240

suddendust · 2025-10-13T10:43:38Z

Support for SELECT on Nested JSON Fields

Summary

Adds support for selecting nested fields from JSONB columns in PostgreSQL flat collections using a new JsonIdentifierExpression type.

Scope: STRING and STRING_ARRAY types only.

Key Changes

New: JsonIdentifierExpression for type-aware JSONB field access with field paths (e.g., props->brand, props->seller->city)
Enhanced: FlatPostgresFieldTransformer to translate JSON paths to PostgreSQL -> / ->> operators
Updated: PostgresDataAccessorIdentifierExpressionVisitor to handle JSON type to PostgreSQL type conversion
Extended: PostgresColTransformer for type-aware casting on JSONB fields

Limitations

Only STRING and STRING_ARRAY types are supported for now.

codecov · 2025-10-13T10:45:14Z

Codecov Report

❌ Patch coverage is 93.54839% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.08%. Comparing base (e0ada8e) to head (f93da60).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...y/v1/transformer/FlatPostgresFieldTransformer.java	78.26%	4 Missing and 1 partial ⚠️
...tore/expression/impl/JsonIdentifierExpression.java	95.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #240      +/-   ##
============================================
+ Coverage     79.88%   80.08%   +0.19%     
- Complexity     1100     1134      +34     
============================================
  Files           213      215       +2     
  Lines          5400     5484      +84     
  Branches        473      481       +8     
============================================
+ Hits           4314     4392      +78     
- Misses          760      764       +4     
- Partials        326      328       +2

Flag	Coverage Δ
integration	`80.08% <93.54%> (+0.19%)`	⬆️
unit	`57.34% <87.09%> (+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-14T06:07:08Z

...rc/main/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpression.java

+/**
+ * Expression representing a nested field within a JSONB column in flat Postgres collections.
+ *
+ * <p>Example: JsonIdentifierExpression.of("customAttr", List.of("myAttribute"), "STRING");


For anything more than 2 arguments, especially if 2 of them are of the same type, I'd prefer using a builder (to make the code less-error prone due to the incorrect argument order).

@suresh-prakash Since we've removed the type arg, it only has 2 arguments now, both of different types (so incorrect arg order is not possible).

suresh-prakash · 2025-10-14T06:08:22Z

...rc/main/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpression.java

+
+  String columnName; // e.g., "customAttr" (the actual JSONB column)
+  List<String> jsonPath; // e.g., ["myAttribute"] (path within the JSONB)
+  String jsonType; // "STRING" or "STRING_ARRAY"


Any reason why this would be required? Can the client parse the JSON to whatever type they want? That way, the clients can also handle complex objects.

If we cannot avoid this, I'd make it an enum, instead of a string.

@suresh-prakash The jsonType determines which PostgreSQL operator and casting to use. For example:

-- STRING type: Uses ->> (extracts as TEXT) SELECT props->>'brand' FROM table WHERE props->>'brand' = 'Nike' -- STRING_ARRAY type: Uses -> (keeps as JSONB) SELECT props->'colors' FROM table WHERE props->'colors' @> '["red"]'

This keeps the query generation logic consistent for flat collections consistent with what we have today for nested. Do you see a better approach here (maybe always extract as a json rather than text)?

I am trying to understand how we achieve it today with nested Postgres table without getting the additional type information from the clients/callers.

@suresh-prakash As discussed yesterday, the type isn't needed.

suresh-prakash · 2025-10-14T06:10:50Z

...rc/main/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpression.java

+
+  public static JsonIdentifierExpression of(
+      final String columnName, final List<String> jsonPath, final String jsonType) {
+    // Construct full name for compatibility: "customAttr.myAttribute"


Since these are column names and JSON paths, they are not fed-in via. PreparedStatement. Hence, this could lead to SQL injection attacks. Could you please add sanity checks everywhere? This is applicable all throughout "flat collections", even for regular IdentifierExpression perhaps. For nested collection, it may not have been an issue since the column name and the table name are hardcoded in the library.

@suresh-prakash Any strategies you know of around this? Also, shall we create a separate story for this to not balloon the scope of this store too much?

Simply ensuring the column names and jsonPaths do not contain any special characters other than an underscore and dot might be a good one to start with.

shall we create a separate story for this to not balloon the scope of this store too much?

When it comes to security aspects, I guess, it's better to act on it as soon as possible. I don't think the work involved would be much anyways.

@suresh-prakash Gotcha!

suresh-prakash · 2025-10-14T06:11:42Z

...rc/main/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpression.java

+@EqualsAndHashCode(callSuper = true)
+public class JsonIdentifierExpression extends IdentifierExpression {
+
+  String columnName; // e.g., "customAttr" (the actual JSONB column)


Isn't this already a part of the parent class?

Yes, it is. Will remove this.

suresh-prakash · 2025-10-14T06:14:25Z

...ypertrace/core/documentstore/postgres/query/v1/transformer/FlatPostgresFieldTransformer.java

+  public FieldToPgColumn transform(
+      IdentifierExpression expression, Map<String, String> pgColMapping) {
+    // Check if this is a JsonIdentifierExpression with explicit metadata
+    if (expression instanceof JsonIdentifierExpression) {


I'd completely avoid the need for instanceof through the visitor pattern as much as possible for a cleaner code structure. The problem with instanceof is whenever we change a logic in one place, we might forget to make a similar change in the other place with instanceof. But, with visitor, all relevant changes are nicely packaged together in a single class (high cohession).

Thanks for the detailed comment! I am using visitors now to avoid the instanceof check.

suresh-prakash · 2025-10-14T06:16:48Z

...documentstore/postgres/query/v1/vistors/PostgresDataAccessorIdentifierExpressionVisitor.java

+
+    // If this is a JsonIdentifierExpression, use its type instead of the visitor's type
+    Type typeToUse = this.type;
+    if (expression instanceof JsonIdentifierExpression) {


Same here.
Having the instanceof checks at different places increases coupling across classes, leading to code maintenance overhead in order to keep it error-free.

suresh-prakash · 2025-10-14T06:18:27Z

...documentstore/postgres/query/v1/vistors/PostgresDataAccessorIdentifierExpressionVisitor.java

+        .buildFieldAccessorWithCast(fieldToPgColumn, typeToUse);
+  }
+
+  private Type convertJsonTypeToPostgresType(String jsonType) {


Why not make the Type enum generic to document store (not specific to Postgres) and re-use it?

I'd prefer avoiding the type at all if possible.

As discussed, we'll get rid of type for now.

This reverts commit 1d2dbb8.

…b_nested_selections

This reverts commit 701b052.

This reverts commit cc47622.

…nto jsonb_nested_selections" This reverts commit 68c7951, reversing changes made to 607f1fc.

…into jsonb_nested_selections" This reverts commit d262937.

suresh-prakash

Everything looks good except https://github.com/hypertrace/document-store/pull/240/files#r2428016668

suddendust · 2025-10-17T08:50:10Z

@suresh-prakash Regarding this, I was thinking of creating a new issue for it, since the changes are at multiple places, blowing up the PR size. Wdyt?

suddendust · 2025-10-17T08:53:19Z

@suresh-prakash Nvm, I just read the other comment. Will be adding this in this PR itself!

suddendust · 2025-10-17T10:05:51Z

@suresh-prakash Can you review: 4a01d1f. We can make the different static vars config driven as well, but for starting, I feel this should be okay?

suddendust added 2 commits October 13, 2025 15:40

Support for SELECT on nested JSON fields

853b162

Test case to validate selections b/w flat and nested PG collections

361a6d2

suddendust requested review from avinashkolluru, kotharironak, skjindal93 and suresh-prakash as code owners October 13, 2025 10:43

suddendust and others added 2 commits October 13, 2025 18:19

Added more test cases

1eb378d

Merge branch 'main' into jsonb_nested_selections

1a669c4

suresh-prakash reviewed Oct 14, 2025

View reviewed changes

suddendust and others added 7 commits October 15, 2025 10:44

Merge branch 'main' into jsonb_nested_selections

3c9ea7a

Visitor for JsonIdentifierExpression

1d2dbb8

Revert "Visitor for JsonIdentifierExpression"

ce89d4e

This reverts commit 1d2dbb8.

WIP

442449d

Added visitors to avoid instanceof checks

607f1fc

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

68c7951

…b_nested_selections

Remove extra comments

cc47622

suddendust changed the title ~~[PoC] Support for SELECT on nested JSON fields~~ Support for SELECT on nested JSON fields Oct 17, 2025

suddendust changed the title ~~Support for SELECT on nested JSON fields~~ Support for SELECT on nested JSON fields in Flat Collections Oct 17, 2025

suddendust added 8 commits October 17, 2025 11:46

Fix compilation issue

701b052

Revert "Fix compilation issue"

e64eb5a

This reverts commit 701b052.

Revert "Remove extra comments"

0da5963

This reverts commit cc47622.

Revert "Merge branch 'main' of github.com:hypertrace/document-store i…

d262937

…nto jsonb_nested_selections" This reverts commit 68c7951, reversing changes made to 607f1fc.

Reapply "Merge branch 'main' of github.com:hypertrace/document-store …

2333877

…into jsonb_nested_selections" This reverts commit d262937.

WIP

ea80660

WIP

73c969a

Added JsonIdentifierExpressionTest.java

af91447

suddendust requested a review from suresh-prakash October 17, 2025 08:20

suresh-prakash reviewed Oct 17, 2025

View reviewed changes

suddendust added 2 commits October 17, 2025 15:34

Add col name validations using PostgresSecurityValidator.java

4a01d1f

Removed extra docs

f93da60

suddendust requested a review from suresh-prakash October 17, 2025 10:05

suresh-prakash approved these changes Oct 17, 2025

View reviewed changes

suresh-prakash merged commit 1e3c868 into hypertrace:main Oct 17, 2025
6 checks passed

Support for SELECT on nested JSON fields in Flat Collections #240

Support for SELECT on nested JSON fields in Flat Collections #240

Uh oh!

Conversation

suddendust commented Oct 13, 2025

Support for SELECT on Nested JSON Fields

Summary

Key Changes

Limitations

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suresh-prakash Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suresh-prakash left a comment

Choose a reason for hiding this comment

Uh oh!

suddendust commented Oct 17, 2025

Uh oh!

suddendust commented Oct 17, 2025

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Oct 13, 2025 •

edited

Loading

suresh-prakash Oct 17, 2025 •

edited

Loading

suddendust commented Oct 17, 2025 •

edited

Loading