Decouple PPL logical plans from UDT temporal types#5547
Conversation
penghuo
force-pushed
the
feat/expr_lazy_udt_v1
branch
from
d17aa3b to
3244bed
Compare
penghuo
force-pushed
the
feat/expr_lazy_udt_v1
branch
from
3244bed to
666bec2
Compare
PR Reviewer Guide 🔍(Review updated until commit aa01135)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to aa01135 Explore these optional code suggestions:
Previous suggestionsSuggestions up to commit c3fc9ca
Suggestions up to commit 60a2c71
Suggestions up to commit 683bc8a
Suggestions up to commit 24cf40c
Suggestions up to commit 3346e5a
|
penghuo
force-pushed
the
feat/expr_lazy_udt_v1
branch
2 times, most recently
from
2f9778b to
e90ffd3
Compare
|
Persistent review updated to latest commit e90ffd3 |
penghuo
force-pushed
the
feat/expr_lazy_udt_v1
branch
from
e90ffd3 to
b9d68eb
Compare
|
Persistent review updated to latest commit b9d68eb |
penghuo
force-pushed
the
feat/expr_lazy_udt_v1
branch
from
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).
penghuo
force-pushed
the
feat/expr_lazy_udt_v1
branch
2 times, most recently
from
24cf40c to
683bc8a
Compare
|
Persistent review updated to latest commit 683bc8a |
penghuo
force-pushed
the
feat/expr_lazy_udt_v1
branch
2 times, most recently
from
60a2c71 to
c3fc9ca
Compare
|
Persistent review updated to latest commit c3fc9ca |
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>
penghuo
force-pushed
the
feat/expr_lazy_udt_v1
branch
from
c3fc9ca to
aa01135
Compare
|
Persistent review updated to latest commit aa01135 |
1 participant
Description
This PR makes two related but distinct architectural changes to the Calcite-side PPL planner. Together they let logical RelNode/RexNode trees produced during analysis use ordinary Calcite types end-to-end. UDTs (and
ExprType) only re-enter the picture immediately before physical execution, where they're needed to keep Linq4j codegen seeing the existing VARCHAR-backed runtime representation.1. Decouple
ExprTypefrom the logical planCalcite-side PPL planning code (RelNode/RexNode construction, coercion, type checking, UDF return-type inference, UDF implementors) now operates on
RelDataTypedirectly instead of converting to/fromExprTypeat every step.CoercionUtils): newRelDataType-typed common-type resolver with an internal widening DAG keyed by a smallCoercionTagenum (matches v2 widening semantics —STRING → TIMESTAMPdirect edge,UNDEFINED → any-concreteunit cost, etc).PPLTypeChecker): signatures expressed asList<List<RelDataType>>; newrenderTypeNamefor error messages; newtemporalKindhelper that canonicalizes DATE/TIME/TIMESTAMP across UDT and standard SQL temporal forms in a single place.PPLOperandTypes,UDFOperandMetadata): signature constants exposed asRelDataType(e.g.INTEGER_T,STRING_T,IP_UDT).AddSubDate,Extract,Format,LastDay,PeriodName,TimestampAdd,TimestampDiff,Weekday,Span,WidthBucket,IPFunction,CidrMatchFunction,CompareIpFunction, geo-IP,CurrentFunction): branch onRelDataType(UDT class instances orSqlTypeName) and passRelDataTypethrough their lowering, instead of bouncing throughExprType.CalciteRexNodeVisitor.visitCast: maps ASTDataTypedirectly toRelDataType, removing theDataType.getCoreType()round-trip.ExtendedRexBuilder: type discrimination viainstanceofUDT classes andSqlTypeName, noExprTypeconversion.UDT identity is preserved at planning time via
instanceofchecks (notgetExprType()); subclasses survivecreateTypeWithNullability/createTypeWithCharsetAndCollationvia acloneWithhook onExprSqlType/ExprJavaType.2. Remove DATE/TIME/TIMESTAMP UDTs from the logical plan
After the
ExprTypedecoupling lands, the only thing pinning UDT temporal types to the logical layer wasconvertExprTypeToRelDataType. This change pushes those UDTs out of the logical plan entirely:OpenSearchTypeFactory.convertExprTypeToRelDataTypereturns standardDATE/TIME(9)/TIMESTAMP(9)for the correspondingExprCoreType. NewisStandardTemporalTypehelper. IP and BINARY remain UDT.CalciteRexNodeVisitor.visitCast): emits standard temporal types for ASTDATE/TIME/TIMESTAMPcast targets.ExtendedRexBuilder.makeCast): standard temporal targets dispatch toPPLBuiltinOperators.DATE/TIME/TIMESTAMPUDFs but keep the call'sRelDataTypestandard. IP routed separately.PredicateAnalyzer):isTimestamp/isDateaccept both standardSqlTypeNameand UDT, and read literal values viagetValueAs(String.class)soTimestampString/DateStringround-trip withoutClassCastException.NULLABLE_*_UDT→NULLABLE_*_T,DATE_UDT/TIME_UDT/TIMESTAMP_UDT→DATE_T/TIME_T/TIMESTAMP_T.DatetimeExtension/DatetimeUdtNormalizeRuleremoved (introduced in Normalize datetime types for unified query API #5408). Their sole purpose was to rewrite UDT temporal return types to standard Calcite types as a unified-API post-analysis pass; the analysis pipeline no longer produces temporal UDTs in the first place, so the rule is a no-op and the extension is removed along with its registrations inUnifiedPplSpec/UnifiedSqlSpec.TemporalUdtRewriteShuttle(the conversion boundary)A new
RelShuttleruns once at the prepare-statement boundary (OpenSearchCalcitePreparingStmt.implementandOpenSearchRelRunners.run) and rewrites every standard temporal type in the tree back to its UDT counterpart. Implementation notes:RelShuttleImpl.visitChildrebuilds parents viaparent.copy(traits, newInputs)after visiting each child, which makes both rewrite-then-visit and visit-then-rewrite orderings briefly violate row-type consistency. The shuttle interceptsProject/Filter/Calc/Aggregate/Valuesand rebuilds them in one shot with rewritten inputs and rewritten RexNodes together.Filterrebuild preservesvariablesSet. Without this, correlated subqueries lose theirCorrelationIdbinding.RexShuttleextensions:visitSubQueryrecurses into the inner plan (Calcite's default doesn't);visitLambda/visitLambdaRefrebuild the lambda body and parameter refs together sotransform(arr, x -> ...)over temporal-element arrays stays consistent.rewriteTyperecurses intoARRAY/MULTISET/MAP/struct, so collection-typed temporal columns (e.g.TAKE(@timestamp, 2)) get rewritten.TemporalSchemaRewritablemarker interface.AbstractCalciteIndexScanimplements it and rewrites its schema in place viabuildScan(...), rather than wrapping in aLogicalProject(CAST(...)). This is what keeps Linq4j codegen readingStringvalues consistent with whatOpenSearchExprValueFactorydelivers at runtime.Other adjustments
explain_appendpipe_command.json× 2 profiles): logical plans now showTIMESTAMP(9)instead ofEXPR_TIMESTAMP VARCHAR; physical plans still show UDT post-shuttle.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.