Introduce Flat Collection Types by suddendust · Pull Request #259 · hypertrace/document-store

suddendust · 2025-12-12T07:51:52Z

Description

This PR introduces a database-agnostic type system that can be used to provide compile time type metadata to query parsers, which can use this info to generate type-aware optimisations.

Testing

[x] Added UTs
[x] Tested this in a live env

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

…CollectionTypes

codecov · 2025-12-12T08:00:36Z

Codecov Report

❌ Patch coverage is 92.64706% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.46%. Comparing base (4516200) to head (3f6957b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
.../parser/filter/nonjson/field/PostgresDataType.java	88.46%	2 Missing and 1 partial ⚠️
...e/expression/impl/AliasedIdentifierExpression.java	0.00%	0 Missing and 1 partial ⚠️
...entstore/expression/impl/IdentifierExpression.java	93.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #259      +/-   ##
============================================
- Coverage     80.48%   80.46%   -0.02%     
- Complexity     1297     1330      +33     
============================================
  Files           230      231       +1     
  Lines          5948     6000      +52     
  Branches        536      537       +1     
============================================
+ Hits           4787     4828      +41     
- Misses          797      805       +8     
- Partials        364      367       +3

Flag	Coverage Δ
integration	`80.46% <92.64%> (-0.02%)`	⬇️
unit	`58.36% <92.64%> (+0.39%)`	⬆️

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 · 2025-12-12T11:21:55Z

LGTM

suresh-prakash

Partially reviewed (till JsonIdentifierExpression.java)

suresh-prakash · 2025-12-15T12:11:21Z

...main/java/org/hypertrace/core/documentstore/expression/impl/AliasedIdentifierExpression.java


  private AliasedIdentifierExpression(final String name, final String contextAlias) {
-    super(name);
+    super(name, null);


This change is not required?

suresh-prakash · 2025-12-15T12:12:04Z

...c/main/java/org/hypertrace/core/documentstore/expression/impl/ArrayIdentifierExpression.java

-    this.arrayType = arrayType;
+  // Package-private: used internally by factory methods
+  ArrayIdentifierExpression(String name, FlatCollectionDataType arrayElementType) {
+    super(name, null);


Can simply invoke the base super(name); constructor?

suresh-prakash · 2025-12-16T05:39:58Z

...c/main/java/org/hypertrace/core/documentstore/expression/impl/ArrayIdentifierExpression.java

+    return of(name, PostgresDataType.TIMESTAMP);
+  }
+
+  public static ArrayIdentifierExpression ofTimestampsTz(final String name) {


This class is supposed to be agnostic of the underlying database. Using Postgres specific-types here breaks the abstraction.
I'm especially concerned since we have a ofTimestamp() and ofTimestampsTz().
This might create a tight coupling.

If necessary, we should have abstracted type-definitions for document-store, which can later map to the database type-specific types during the query translation.

If we want to make it ideal, we should keep PostgresDataType at the module/library level.

@suresh-prakash Sure, added generic types to remove this coupling. We've specific extractors for PG now that do this mapping.

suresh-prakash · 2025-12-16T05:41:52Z

...c/main/java/org/hypertrace/core/documentstore/expression/impl/ArrayIdentifierExpression.java

+    return of(name, PostgresDataType.DATE);
+  }
+
+  public static ArrayIdentifierExpression ofUuids(final String name) {


Do we need to create a public method for every Postgres type here? I'd start with minimal types what is required (and common across different database types) and live with it and start adding more on a need basis.

Removed unnecessary type like UUID but kept TIMESTAMPZ and DATE (if feel we might use these types more than UUID, so I don't want to make a new code change to introduce these types later).

suresh-prakash · 2025-12-16T05:44:42Z

...c/main/java/org/hypertrace/core/documentstore/expression/impl/ArrayIdentifierExpression.java

+    return getArrayElementType().map(this::toPostgresArrayType);
+  }
+
+  private String toPostgresArrayType(FlatCollectionDataType type) {


Having the name "toPostgres..." in a generic class breaks abstraction though it is private. If it's possible, I'd avoid it.

Added generic types.

suresh-prakash · 2025-12-16T05:45:57Z

...re/src/main/java/org/hypertrace/core/documentstore/expression/impl/IdentifierExpression.java

  public static IdentifierExpression of(final String name) {
    Preconditions.checkArgument(name != null && !name.isBlank(), "name is null or blank");
-    return new IdentifierExpression(name);
+    return new IdentifierExpression(name, null);


Instead of having to deal with nulls, it'd be better to introduce a "safe" enum (like how proto. does).

Introduced a UNSPECIFIED type here.

suresh-prakash · 2025-12-16T05:46:42Z

...re/src/main/java/org/hypertrace/core/documentstore/expression/impl/IdentifierExpression.java


  String name;
+  // Type information of this identifier for flat collections, optional
+  FlatCollectionDataType flatCollectionDataType;


If it's nullable, better to annotate it as @Nullable so that we get compilation issues.

This is no longer a part of the public API so don't think this is needed anymore.

suresh-prakash · 2025-12-16T05:47:07Z

...re/src/main/java/org/hypertrace/core/documentstore/expression/impl/IdentifierExpression.java

+  }
+
+  public static IdentifierExpression ofTimestamp(final String name) {
+    return of(name, PostgresDataType.TIMESTAMP);


suresh-prakash · 2025-12-16T05:47:55Z

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

  protected JsonIdentifierExpression(
      String name, String columnName, List<String> jsonPath, JsonFieldType fieldType) {
-    super(name);
+    super(name, null);


Can we avoiding these changes.

@suresh-prakash Yes.

suresh-prakash · 2025-12-18T03:41:36Z

document-store/src/main/java/org/hypertrace/core/documentstore/expression/impl/DataType.java

+  FLOAT,
+  DOUBLE,
+  BOOLEAN,
+  TIMESTAMPTZ,


Can you add a comment on this enum? It is not evident what the TZ suffix represents.

suresh-prakash · 2025-12-18T03:42:27Z

...re/src/main/java/org/hypertrace/core/documentstore/expression/impl/IdentifierExpression.java

+    return of(name, DataType.BOOLEAN);
+  }
+
+  public static IdentifierExpression ofTimestampTz(final String name) {


Either a documentation comment will be helpful, or would be better to drop the Tz suffix.

Added doc for this.

Introduce flat collection types

8375ac6

suddendust requested review from avinashkolluru, kotharironak, skjindal93 and suresh-prakash as code owners December 12, 2025 07:51

suddendust changed the title ~~Introduce Flat Collection Types~~ [Draft] Introduce Flat Collection Types Dec 12, 2025

suddendust added 2 commits December 12, 2025 13:22

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

193c232

…CollectionTypes

WIP

5bd4de7

suddendust added 3 commits December 12, 2025 16:32

Add bytea data type

6607b21

Added more UTs

8a06c95

Added more UTs

777559f

suddendust changed the title ~~[Draft] Introduce Flat Collection Types~~ Introduce Flat Collection Types Dec 12, 2025

suddendust added 2 commits December 12, 2025 16:55

Added more UTs

42522c5

Added more UTs

3083cc1

suresh-prakash reviewed Dec 16, 2025

View reviewed changes

Introduces abstract types for flat collections, address other comments

5e76e5e

suddendust requested a review from suresh-prakash December 17, 2025 08:28

suddendust added 2 commits December 17, 2025 14:00

Removed inadvertent changes

cbf274e

Fixed failing test case

47025d5

suresh-prakash previously approved these changes Dec 18, 2025

View reviewed changes

Added docs

3f6957b

suddendust dismissed suresh-prakash’s stale review via 3f6957b December 18, 2025 06:02

suddendust requested a review from suresh-prakash December 18, 2025 06:02

suresh-prakash approved these changes Dec 18, 2025

View reviewed changes

suresh-prakash merged commit 78dc7c1 into hypertrace:main Dec 18, 2025
6 checks passed

Conversation

suddendust commented Dec 12, 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 Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

puneet-traceable commented Dec 12, 2025

Uh oh!

suresh-prakash left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

suddendust commented Dec 12, 2025 •

edited

Loading

codecov bot commented Dec 12, 2025 •

edited

Loading