-
Notifications
You must be signed in to change notification settings - Fork 11
Introduce Flat Collection Types #259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8375ac6
193c232
5bd4de7
6607b21
8a06c95
777559f
42522c5
3083cc1
5e76e5e
cbf274e
47025d5
3f6957b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package org.hypertrace.core.documentstore.expression.impl; | ||
|
|
||
| /** | ||
| * Database-agnostic data types for explicit type annotation in queries. | ||
| * | ||
| * <p>This enum provides type metadata for {@link IdentifierExpression} and {@link | ||
| * ArrayIdentifierExpression} fields in flat collections, enabling type-safe query generation | ||
| * without runtime type inference. | ||
| * | ||
| * <p>These types are mapped to database-specific types at query parsing time. For example, when | ||
| * generating PostgreSQL queries, {@code STRING} maps to {@code text}, {@code INTEGER} maps to | ||
| * {@code int4}, etc. | ||
| * | ||
| * @see ArrayIdentifierExpression | ||
| * @see IdentifierExpression | ||
| */ | ||
| public enum DataType { | ||
| UNSPECIFIED, | ||
| STRING, | ||
| INTEGER, | ||
| LONG, | ||
| FLOAT, | ||
| DOUBLE, | ||
| BOOLEAN, | ||
| // timestamp with time-zone information. For example: 2004-10-19 10:23:54+02. | ||
| // For more info, see: https://www.postgresql.org/docs/current/datatype-datetime.html | ||
| TIMESTAMPTZ, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a comment on this enum? It is not evident what the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added doc. |
||
| DATE | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,11 @@ | |
| * Expression representing either an identifier/column name | ||
| * | ||
| * <p>Example: IdentifierExpression.of("col1"); | ||
| * | ||
| * <p>For flat relational collections, you can optionally provide a {@link DataType} to enable | ||
| * type-safe query generation without runtime type inference: | ||
| * | ||
| * <p>Example: IdentifierExpression.ofInt("price"); | ||
| */ | ||
| @Value | ||
| @NonFinal | ||
|
|
@@ -25,12 +30,59 @@ public class IdentifierExpression | |
| implements GroupTypeExpression, SelectTypeExpression, SortTypeExpression { | ||
|
|
||
| String name; | ||
| // Type information of this identifier for flat collections, this is optional to maintain backward | ||
| // compatibility | ||
| DataType dataType; | ||
|
|
||
| IdentifierExpression(String name) { | ||
| this.name = name; | ||
| this.dataType = DataType.UNSPECIFIED; | ||
| } | ||
|
|
||
| public static IdentifierExpression of(final String name) { | ||
| Preconditions.checkArgument(name != null && !name.isBlank(), "name is null or blank"); | ||
| return new IdentifierExpression(name); | ||
| } | ||
|
|
||
| static IdentifierExpression of(final String name, final DataType dataType) { | ||
| Preconditions.checkArgument(name != null && !name.isBlank(), "name is null or blank"); | ||
| return new IdentifierExpression(name, dataType); | ||
| } | ||
|
|
||
| public static IdentifierExpression ofString(final String name) { | ||
| return of(name, DataType.STRING); | ||
| } | ||
|
|
||
| public static IdentifierExpression ofInt(final String name) { | ||
| return of(name, DataType.INTEGER); | ||
| } | ||
|
|
||
| public static IdentifierExpression ofLong(final String name) { | ||
| return of(name, DataType.LONG); | ||
| } | ||
|
|
||
| public static IdentifierExpression ofFloat(final String name) { | ||
| return of(name, DataType.FLOAT); | ||
| } | ||
|
|
||
| public static IdentifierExpression ofDouble(final String name) { | ||
| return of(name, DataType.DOUBLE); | ||
| } | ||
|
|
||
| public static IdentifierExpression ofBoolean(final String name) { | ||
| return of(name, DataType.BOOLEAN); | ||
| } | ||
|
|
||
| // Timestamp with time-zone information. For example: 2004-10-19 10:23:54+02. For more info, see: | ||
| // https://www.postgresql.org/docs/current/datatype-datetime.html | ||
| public static IdentifierExpression ofTimestampTz(final String name) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Either a documentation comment will be helpful, or would be better to drop the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added doc for this. |
||
| return of(name, DataType.TIMESTAMPTZ); | ||
| } | ||
|
|
||
| public static IdentifierExpression ofDate(final String name) { | ||
| return of(name, DataType.DATE); | ||
| } | ||
|
|
||
| @Override | ||
| public <T> T accept(final GroupTypeExpressionVisitor visitor) { | ||
| return visitor.visit(this); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,5 +9,6 @@ public enum JsonFieldType { | |
| NUMBER_ARRAY, | ||
| BOOLEAN_ARRAY, | ||
| OBJECT_ARRAY, | ||
| OBJECT | ||
| OBJECT, | ||
| UNSPECIFIED | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.nonjson.field; | ||
|
|
||
| import org.hypertrace.core.documentstore.expression.impl.DataType; | ||
|
|
||
| /** | ||
| * PostgreSQL-specific data types with their SQL type strings. | ||
| * | ||
| * <p>This enum maps generic {@link DataType} values to PostgreSQL-specific type strings used in SQL | ||
| * queries for type casting. | ||
| */ | ||
| public enum PostgresDataType { | ||
| TEXT("text"), | ||
| INTEGER("integer"), | ||
| BIGINT("bigint"), | ||
| REAL("real"), | ||
| DOUBLE_PRECISION("double precision"), | ||
| BOOLEAN("boolean"), | ||
| TIMESTAMPTZ("timestamptz"), | ||
| DATE("date"), | ||
| UNKNOWN("unknown"); | ||
|
|
||
| private final String sqlType; | ||
|
|
||
| PostgresDataType(String sqlType) { | ||
| this.sqlType = sqlType; | ||
| } | ||
|
|
||
| public String getSqlType() { | ||
| return sqlType; | ||
| } | ||
|
|
||
| public String getArraySqlType() { | ||
| return sqlType + "[]"; | ||
| } | ||
|
|
||
| /** | ||
| * Maps a generic DataType to its PostgreSQL equivalent. | ||
| * | ||
| * @param dataType the generic data type | ||
| * @return the corresponding PostgresDataType, or null if UNSPECIFIED | ||
| * @throws IllegalArgumentException if the DataType is unknown | ||
| */ | ||
| public static PostgresDataType fromDataType(DataType dataType) { | ||
| switch (dataType) { | ||
| case UNSPECIFIED: | ||
| return UNKNOWN; | ||
| case STRING: | ||
| return TEXT; | ||
| case INTEGER: | ||
| return INTEGER; | ||
| case LONG: | ||
| return BIGINT; | ||
| case FLOAT: | ||
| return REAL; | ||
| case DOUBLE: | ||
| return DOUBLE_PRECISION; | ||
| case BOOLEAN: | ||
| return BOOLEAN; | ||
| case TIMESTAMPTZ: | ||
| return TIMESTAMPTZ; | ||
| case DATE: | ||
| return DATE; | ||
| default: | ||
| throw new IllegalArgumentException("Unknown DataType: " + dataType); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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()andofTimestampsTz().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
PostgresDataTypeat the module/library level.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@suresh-prakash Sure, added generic types to remove this coupling. We've specific extractors for PG now that do this mapping.