Decouple PPL logical plans from UDT temporal types#5547
Conversation
d17aa3b to
3244bed
Compare
3244bed to
666bec2
Compare
PR Reviewer Guide 🔍(Review updated until commit 429010b)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to b37b1f5 Explore these optional code suggestions:
Previous suggestionsSuggestions up to commit f5d1237
Suggestions up to commit 429010b
Suggestions up to commit aa01135
Suggestions up to commit c3fc9ca
Suggestions up to commit 60a2c71
|
2f9778b to
e90ffd3
Compare
|
Persistent review updated to latest commit e90ffd3 |
e90ffd3 to
b9d68eb
Compare
|
Persistent review updated to latest commit b9d68eb |
b9d68eb to
3346e5a
Compare
| { | ||
| "calcite": { | ||
| "logical":"LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])\n LogicalProject(account_number=[$0], firstname=[$1], address=[$2], birthdate=[$3], gender=[$4], city=[$5], lastname=[$6], balance=[$7], employer=[$8], state=[$9], age=[$10], email=[$11], male=[$12], cnt=[$19])\n LogicalUnion(all=[true])\n LogicalProject(account_number=[$0], firstname=[$1], address=[$2], birthdate=[$3], gender=[$4], city=[$5], lastname=[$6], balance=[$7], employer=[$8], state=[$9], age=[$10], email=[$11], male=[$12], _id=[$13], _index=[$14], _score=[$15], _maxscore=[$16], _sort=[$17], _routing=[$18], cnt=[null:BIGINT])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]])\n LogicalProject(account_number=[null:BIGINT], firstname=[null:VARCHAR], address=[null:VARCHAR], birthdate=[null:EXPR_TIMESTAMP VARCHAR], gender=[$0], city=[null:VARCHAR], lastname=[null:VARCHAR], balance=[null:BIGINT], employer=[null:VARCHAR], state=[null:VARCHAR], age=[null:INTEGER], email=[null:VARCHAR], male=[null:BOOLEAN], _id=[null:VARCHAR], _index=[null:VARCHAR], _score=[null:REAL], _maxscore=[null:REAL], _sort=[null:BIGINT], _routing=[null:VARCHAR], cnt=[$1])\n LogicalAggregate(group=[{0}], cnt=[COUNT($1)])\n LogicalProject(gender=[$4], balance=[$7])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]])\n", | ||
| "logical":"LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])\n LogicalProject(account_number=[$0], firstname=[$1], address=[$2], birthdate=[$3], gender=[$4], city=[$5], lastname=[$6], balance=[$7], employer=[$8], state=[$9], age=[$10], email=[$11], male=[$12], cnt=[$19])\n LogicalUnion(all=[true])\n LogicalProject(account_number=[$0], firstname=[$1], address=[$2], birthdate=[$3], gender=[$4], city=[$5], lastname=[$6], balance=[$7], employer=[$8], state=[$9], age=[$10], email=[$11], male=[$12], _id=[$13], _index=[$14], _score=[$15], _maxscore=[$16], _sort=[$17], _routing=[$18], cnt=[null:BIGINT])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]])\n LogicalProject(account_number=[null:BIGINT], firstname=[null:VARCHAR], address=[null:VARCHAR], birthdate=[null:TIMESTAMP(9)], gender=[$0], city=[null:VARCHAR], lastname=[null:VARCHAR], balance=[null:BIGINT], employer=[null:VARCHAR], state=[null:VARCHAR], age=[null:INTEGER], email=[null:VARCHAR], male=[null:BOOLEAN], _id=[null:VARCHAR], _index=[null:VARCHAR], _score=[null:REAL], _maxscore=[null:REAL], _sort=[null:BIGINT], _routing=[null:VARCHAR], cnt=[$1])\n LogicalAggregate(group=[{0}], cnt=[COUNT($1)])\n LogicalProject(gender=[$4], balance=[$7])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]])\n", |
There was a problem hiding this comment.
birthdate=[null:EXPR_TIMESTAMP VARCHAR] -> birthdate=[null:TIMESTAMP(9)]
This is expected change. Logical plan date type should be TIMESTAMP(9).
| { | ||
| "calcite": { | ||
| "logical":"LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])\n LogicalProject(account_number=[$0], firstname=[$1], address=[$2], birthdate=[$3], gender=[$4], city=[$5], lastname=[$6], balance=[$7], employer=[$8], state=[$9], age=[$10], email=[$11], male=[$12], cnt=[$19])\n LogicalUnion(all=[true])\n LogicalProject(account_number=[$0], firstname=[$1], address=[$2], birthdate=[$3], gender=[$4], city=[$5], lastname=[$6], balance=[$7], employer=[$8], state=[$9], age=[$10], email=[$11], male=[$12], _id=[$13], _index=[$14], _score=[$15], _maxscore=[$16], _sort=[$17], _routing=[$18], cnt=[null:BIGINT])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]])\n LogicalProject(account_number=[null:BIGINT], firstname=[null:VARCHAR], address=[null:VARCHAR], birthdate=[null:EXPR_TIMESTAMP VARCHAR], gender=[$0], city=[null:VARCHAR], lastname=[null:VARCHAR], balance=[null:BIGINT], employer=[null:VARCHAR], state=[null:VARCHAR], age=[null:INTEGER], email=[null:VARCHAR], male=[null:BOOLEAN], _id=[null:VARCHAR], _index=[null:VARCHAR], _score=[null:REAL], _maxscore=[null:REAL], _sort=[null:BIGINT], _routing=[null:VARCHAR], cnt=[$1])\n LogicalAggregate(group=[{0}], cnt=[COUNT($1)])\n LogicalProject(gender=[$4], balance=[$7])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]])\n", | ||
| "logical":"LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])\n LogicalProject(account_number=[$0], firstname=[$1], address=[$2], birthdate=[$3], gender=[$4], city=[$5], lastname=[$6], balance=[$7], employer=[$8], state=[$9], age=[$10], email=[$11], male=[$12], cnt=[$19])\n LogicalUnion(all=[true])\n LogicalProject(account_number=[$0], firstname=[$1], address=[$2], birthdate=[$3], gender=[$4], city=[$5], lastname=[$6], balance=[$7], employer=[$8], state=[$9], age=[$10], email=[$11], male=[$12], _id=[$13], _index=[$14], _score=[$15], _maxscore=[$16], _sort=[$17], _routing=[$18], cnt=[null:BIGINT])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]])\n LogicalProject(account_number=[null:BIGINT], firstname=[null:VARCHAR], address=[null:VARCHAR], birthdate=[null:TIMESTAMP(9)], gender=[$0], city=[null:VARCHAR], lastname=[null:VARCHAR], balance=[null:BIGINT], employer=[null:VARCHAR], state=[null:VARCHAR], age=[null:INTEGER], email=[null:VARCHAR], male=[null:BOOLEAN], _id=[null:VARCHAR], _index=[null:VARCHAR], _score=[null:REAL], _maxscore=[null:REAL], _sort=[null:BIGINT], _routing=[null:VARCHAR], cnt=[$1])\n LogicalAggregate(group=[{0}], cnt=[COUNT($1)])\n LogicalProject(gender=[$4], balance=[$7])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]])\n", |
There was a problem hiding this comment.
birthdate=[null:EXPR_TIMESTAMP VARCHAR] -> birthdate=[null:TIMESTAMP(9)]
This is expected change. Logical plan date type should be TIMESTAMP(9).
24cf40c to
683bc8a
Compare
|
Persistent review updated to latest commit 683bc8a |
60a2c71 to
c3fc9ca
Compare
|
Persistent review updated to latest commit c3fc9ca |
c3fc9ca to
aa01135
Compare
|
Persistent review updated to latest commit aa01135 |
aa01135 to
429010b
Compare
|
Persistent review updated to latest commit 429010b |
429010b to
f5d1237
Compare
Logical RelNode/RexNode trees now use standard Calcite DATE/TIME(9)/ TIMESTAMP(9) types for date columns. A new TemporalUdtRewriteShuttle runs at the prepare-statement boundary (OpenSearchCalcitePreparingStmt .implement and OpenSearchRelRunners.run) and rewrites the standard temporal types back to UDTs (ExprDateType/ExprTimeType/ExprTimeStampType) just before physical execution, so Linq4j keeps receiving the VARCHAR-backed representation. This commit folds in the prior "Decouple Calcite PPL planning from ExprType" change as well: Calcite-side PPL planning (RelNode/RexNode code, coercion, type checking, and UDF implementations) operates on RelDataType throughout instead of bouncing through ExprType. Type system: - OpenSearchTypeFactory.convertExprTypeToRelDataType returns standard TIMESTAMP(9)/DATE/TIME(9) for the corresponding ExprCoreType. IP and BINARY remain UDT. - New helper isStandardTemporalType. - PPLOperandTypes constants renamed DATE_UDT/TIME_UDT/TIMESTAMP_UDT to DATE_T/TIME_T/TIMESTAMP_T. - UserDefinedFunctionUtils NULLABLE_*_UDT renamed to NULLABLE_*_T and repointed to standard temporal RelDataTypes (IP and BINARY constants unchanged). - UDTs are recognised via instanceof rather than getExprType(); cloneWith preserves UDT identity through createTypeWithNullability. Cast emission and pushdown: - CalciteRexNodeVisitor.visitCast emits standard temporal types for AST DATE/TIME/TIMESTAMP cast targets. - ExtendedRexBuilder.makeCast routes IP separately; standard temporal targets dispatch to PPLBuiltinOperators.DATE/TIME/TIMESTAMP with the standard target type so the call's RelDataType stays standard. - PredicateAnalyzer.isTimestamp/isDate accept both standard SqlTypeName and UDT, and read literal values via getValueAs(String.class) so TimestampString/DateString round-trip without ClassCastException. Coercion + type-checker: - RelDataType-typed common-type resolver with a CoercionTag widening DAG. - CoercionUtils' DATE+TIME -> TIMESTAMP resolver emits standard TIMESTAMP(9). - PPLTypeChecker gains a temporalKind helper used in typesMatch and isComparable so standard and UDT temporal pairs match across forms. - getRelDataTypes(family) returns standard temporal types for DATETIME/TIMESTAMP/DATE/TIME families (BINARY family stays UDT). - UDF return-type inference (AddSubDate/Weekday/LastDay/TimestampDiff /Format/TimestampAdd/Extract/Span/WidthBucket) recognise both standard and UDT temporal operand types. Shuttle implementation: - Atomic rebuild path for Project/Filter/Calc/Aggregate/Values nodes (Calcite's default copy-then-validate path doesn't work because each half of the rewrite would briefly violate row-type consistency). - Filter rebuild preserves variablesSet so correlated subqueries keep their CorrelationId binding. - TemporalSchemaRewritable marker interface lets OpenSearch table scans rewrite their schema in place rather than wrap in a LogicalProject(CAST), so Calcite Linq4j codegen reads String values matching what OpenSearchExprValueFactory delivers at runtime. Other: - DatetimeUdtNormalizeRule (api/datetime extension) recognises both UDT and standard temporal RexCalls and forces precision to MAX so unified-API consumers see TIMESTAMP(9). - Calcite IT golden files updated to reflect logical plans now showing TIMESTAMP(9) instead of EXPR_TIMESTAMP VARCHAR (physical plans still show UDT post-shuttle). Tests: - New unit: TemporalUdtRewriteShuttleTest, CalcitePPLLogicalPlanStandardTemporalTest. - Updated CoercionUtilsTest, OpenSearchTypeFactoryTest. All module unit tests pass. Full Calcite IT suite (including ExplainIT) green. Signed-off-by: Peng Huo <penghuo@gmail.com>
f5d1237 to
b37b1f5
Compare
|
close this PR. |
1 participant
Description
This PR makes two related but distinct architectural changes to the Calcite-side PPL planner. Scope: DATE/TIME/TIMESTAMP only. IP/BINARY UDTs and ExprType usage in UDF runtime and pushdown paths are intentionally unchanged.
1. Decouple `ExprType` from the Calcite planning surface. Coercion (`CoercionUtils`), type checking (`PPLTypeChecker`), UDF operand metadata (`PPLOperandTypes`), UDF return-type inference, `visitCast`, and `ExtendedRexBuilder` now operate on `RelDataType` directly instead of converting to/from `ExprType` at every step.
2. Remove DATE/TIME/TIMESTAMP UDTs from logical plans. `convertExprTypeToRelDataType` returns standard `DATE`/`TIME(9)`/`TIMESTAMP(9)` for temporal columns. A new `TemporalUdtRewriteShuttle` runs at the prepare-statement boundary (three entry points) and rewrites those back to UDTs just before physical execution, so Linq4j codegen keeps seeing the VARCHAR-backed representation that `OpenSearchExprValueFactory` delivers. `AbstractCalciteIndexScan` rewrites its schema in place via a `TemporalSchemaRewritable` marker interface. `DatetimeExtension` + `DatetimeUdtNormalizeRule` (from #5408) are removed since the analysis pipeline no longer produces temporal UDTs. Nullability and format-string precision (TIMESTAMP(9)) preserved via coercion-cast tweaks.
Related Issues
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.