[analytics-engine] Rewrite engine-output CAST(TIMESTAMP AS VARCHAR) to to_char

[mengweieric]

Fixes opensearch-project/sql#5420.

Problem

PPL's documented timestamp wire format uses a space separator ("2024-01-15 12:00:00"). On the analytics-engine route, DatetimeOutputCastRule wraps every output datetime field in CAST(<datetime> AS VARCHAR) so the unified planner stays backend-agnostic. Calcite renders that cast as the documented space-separated form; DataFusion's Arrow CAST kernel renders it ISO-8601 ("2024-01-15T12:00:00"). The datafusion.format.timestamp_format session config only affects DataFusion's CLI display pipeline, not the cast kernel.

Approach

A small pre-Substrait rewrite at the analytics-engine boundary swaps the engine-output cast for an explicit format call:

CAST(<TIMESTAMP> AS VARCHAR)  →  to_char(<TIMESTAMP>, '%Y-%m-%d %H:%M:%S')

Routed to DataFusion's native to_char scalar via the existing Substrait simple-extension mechanism — no new UDFs.

Scope is intentionally narrow:

• Output Project only. Accepts any Project subclass (LogicalProject or OpenSearchProject) and descends through the unified planner's single LogicalSort system-limit wrapper when present. Inner projections round-trip verbatim. Not recursive.
• Direct slots only. Casts buried inside CASE/COALESCE/UDF arguments round-trip verbatim.
• TIMESTAMP → VARCHAR only. DATE/TIME cast cleanly through Arrow already; CHAR(n) and TIMESTAMP_WITH_LOCAL_TIME_ZONE are out of scope.
• Seconds-only format. Matches the contract from DatetimeOutputCastRule's introducing PR (sql#5408): "PPL Calcite produces ANSI SQL format 2024-01-15 12:00:00." TIMESTAMP(9) sources resolve to the same seconds-only output.

Performance

Negligible. Plan-time: a single pass over the root projection's slot list, bounded by SELECT-list width. Runtime: replaces one row-level formatter with another in the same projection — no extra physical stage, same node count, on timestamp output columns only.

Test plan

• DatetimeOutputCastRewriterTests — direct cast rewrite, Sort(Project) wrapper handling, inner projection untouched, nested cast inside CASE untouched, CHAR(n) untouched, DATE/TIME/integer sources untouched, plain field reference untouched, TIMESTAMP(9) source resolves to seconds-only format.
• DataFusionFragmentConvertorTests#testProjectTimestampOutputCastEmitsToCharExtension — end-to-end Substrait serialization asserts the formatted-render extension function (not a raw cast) is emitted.
• ./gradlew check -p sandbox -Dsandbox.enabled=true.
• PPL IT smoke on a force-routed analytics-engine cluster: pre-fix, the canonical now() / current_timestamp() regression class failed with T-separator output; post-fix the same calls return space-separated strings and pass.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit e2f7b3a)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to e2f7b3a

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Validate operands list before access Add null-safety check for call.getOperands() before accessing index 0. If the CAST call has no operands (malformed expression), accessing index 0 will throw IndexOutOfBoundsException. Validate that the operands list is non-empty before proceeding with the rewrite logic. sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/DatetimeOutputCastRewriter.java [164-182] private static RexNode rewriteDirectOutputCast(RexNode expr, RexBuilder rexBuilder) { if (!(expr instanceof RexCall call) || call.getKind() != SqlKind.CAST) { + return expr; + } + if (call.getOperands().isEmpty()) { return expr; } RexNode source = call.getOperands().get(0); SqlTypeName sourceType = source.getType().getSqlTypeName(); SqlTypeName targetType = call.getType().getSqlTypeName(); if (sourceType != SqlTypeName.TIMESTAMP) { return expr; } // VARCHAR-only: DatetimeOutputCastRule emits CAST(... AS VARCHAR) (length-unspecified). // CHAR(n) is user-authored and has length/padding semantics that to_char does not preserve. if (targetType != SqlTypeName.VARCHAR) { return expr; } RelDataType formatType = rexBuilder.getTypeFactory().createSqlType(SqlTypeName.VARCHAR); RexNode formatLiteral = rexBuilder.makeLiteral(PPL_TIMESTAMP_FORMAT, formatType, true); return rexBuilder.makeCall(call.getType(), SqlLibraryOperators.TO_CHAR, List.of(source, formatLiteral)); } Suggestion importance[1-10]: 3 __ Why: While adding a null-safety check for call.getOperands() is technically correct, Calcite's RexCall for CAST operations always has exactly one operand by design. A malformed CAST expression would be caught during earlier query validation phases. This defensive check adds minimal value in practice but doesn't hurt.	Low

---

Previous suggestions

Suggestions up to commit d0057a0

Category	Suggestion	Impact
Possible issue	Validate operand list before access Add null-safety check for call.getOperands() before accessing index 0. If the CAST call has no operands (malformed expression), accessing get(0) will throw IndexOutOfBoundsException. Validate operand list size before dereferencing. sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/DatetimeOutputCastRewriter.java [164-182] private static RexNode rewriteDirectOutputCast(RexNode expr, RexBuilder rexBuilder) { if (!(expr instanceof RexCall call) || call.getKind() != SqlKind.CAST) { + return expr; + } + if (call.getOperands().isEmpty()) { return expr; } RexNode source = call.getOperands().get(0); SqlTypeName sourceType = source.getType().getSqlTypeName(); SqlTypeName targetType = call.getType().getSqlTypeName(); if (sourceType != SqlTypeName.TIMESTAMP) { return expr; } // VARCHAR-only: DatetimeOutputCastRule emits CAST(... AS VARCHAR) (length-unspecified). // CHAR(n) is user-authored and has length/padding semantics that to_char does not preserve. if (targetType != SqlTypeName.VARCHAR) { return expr; } RelDataType formatType = rexBuilder.getTypeFactory().createSqlType(SqlTypeName.VARCHAR); RexNode formatLiteral = rexBuilder.makeLiteral(PPL_TIMESTAMP_FORMAT, formatType, true); return rexBuilder.makeCall(call.getType(), SqlLibraryOperators.TO_CHAR, List.of(source, formatLiteral)); } Suggestion importance[1-10]: 3 __ Why: While adding a null-safety check for call.getOperands() is technically valid defensive programming, Calcite's RexCall for CAST operations always has exactly one operand by design. The check at line 165 already validates call.getKind() != SqlKind.CAST, which ensures the operand list structure is correct. This suggestion addresses an extremely unlikely edge case that would indicate a malformed Calcite expression tree, which should never occur in practice.	Low

Suggestions up to commit 657df0c

Category	Suggestion	Impact
Possible issue	Add operands list empty check Add null-safety check for call.getOperands() before accessing index 0. If the CAST call has no operands (malformed expression), accessing get(0) will throw an IndexOutOfBoundsException. Verify the operands list is non-empty before proceeding with the rewrite logic. sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/DatetimeOutputCastRewriter.java [122-140] private static RexNode rewriteDirectOutputCast(RexNode expr, RexBuilder rexBuilder) { if (!(expr instanceof RexCall call) || call.getKind() != SqlKind.CAST) { + return expr; + } + if (call.getOperands().isEmpty()) { return expr; } RexNode source = call.getOperands().get(0); SqlTypeName sourceType = source.getType().getSqlTypeName(); SqlTypeName targetType = call.getType().getSqlTypeName(); if (sourceType != SqlTypeName.TIMESTAMP) { return expr; } // VARCHAR-only: DatetimeOutputCastRule emits CAST(... AS VARCHAR) (length-unspecified). // CHAR(n) is user-authored and has length/padding semantics that to_char does not preserve. if (targetType != SqlTypeName.VARCHAR) { return expr; } RelDataType formatType = rexBuilder.getTypeFactory().createSqlType(SqlTypeName.VARCHAR); RexNode formatLiteral = rexBuilder.makeLiteral(PPL_TIMESTAMP_FORMAT, formatType, true); return rexBuilder.makeCall(call.getType(), SqlLibraryOperators.TO_CHAR, List.of(source, formatLiteral)); } Suggestion importance[1-10]: 3 __ Why: While adding a null-safety check for empty operands is technically valid defensive programming, Calcite's RexCall for CAST operations always has exactly one operand by design. The check at line 123 already validates call.getKind() != SqlKind.CAST, which ensures the call is a well-formed CAST expression. This suggestion addresses an edge case that should never occur in practice within Calcite's type system, making it a low-impact defensive addition rather than fixing an actual issue.	Low

[github-actions]

❌ Gradle check result for 657df0c: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit d0057a0

[github-actions]

Persistent review updated to latest commit e2f7b3a

[github-actions]

✅ Gradle check result for e2f7b3a: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.45%. Comparing base (3bea28a) to head (e2f7b3a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #21650      +/-   ##
============================================
- Coverage     73.48%   73.45%   -0.03%     
+ Complexity    74736    74713      -23     
============================================
  Files          5983     5983              
  Lines        339062   339062              
  Branches      48882    48882              
============================================
- Hits         249162   249061     -101     
- Misses        70064    70165     +101     
  Partials      19836    19836              

☔ 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.