Normalize datetime types for unified query API by dai-chen · Pull Request #5408 · opensearch-project/sql

dai-chen · 2026-05-06T02:34:16Z

Description

Add postAnalysisRules to LanguageSpec.LanguageExtension and register DatetimeExtension in UnifiedPplSpec with two post-analysis rules:

DatetimeUdtNormalizeRule: Rewrites PPL datetime UDT return types (EXPR_DATE/TIME/TIMESTAMP) on RexCall nodes to standard Calcite types, enabling downstream consumers to process the plan without UDT-related failures.
DatetimeOutputCastRule: Adds a final LogicalProject that casts standard datetime output columns to VARCHAR, aligning with PPL's wire-format contract (ISO string representation).

Examples

Case 1: UDT Normalize Rule — source=events | eval d = DATE(name) | fields d

 Before:  LogicalProject(d=[DATE($1):EXPR_DATE])
 After:   LogicalProject(d=[DATE($1):DATE])

Case 2: Output Cast Rule — source=events | fields hire_date, start_time, created_at

 Before:  LogicalProject(hire_date=[$2], start_time=[$3], created_at=[$4])
            LogicalTableScan(table=[[events]])

 After:   LogicalProject(hire_date=[CAST($0):VARCHAR], start_time=[CAST($1):VARCHAR], created_at=[CAST($2):VARCHAR])
            LogicalProject(hire_date=[$2], start_time=[$3], created_at=[$4])
              LogicalTableScan(table=[[events]])

Notes

The UDT normalize rule rewrites datetime function return types to standard Calcite types, but a mismatch remains with the underlying PPL UDF implementations (which still operate on String values). This does not affect the Analytics Engine path (reimplements UDFs in DataFusion) but will fail on the UnifiedQueryCompiler path. Follow-up PR will address function implementations.
Currently, the output cast rule uses CAST(datetime AS VARCHAR) whose string format is engine-dependent: PPL Calcite produce ANSI SQL format (2024-01-15 12:00:00) like most other SQL databases (SparkSQL, PostgreSQL, MySQL, Oracle, SQL Server), while DataFusion produces ISO 8601 format (2024-01-15T12:00:00).

Related Issues

Part of #5250

Check List

New functionality includes testing.
New functionality has been documented.
New functionality has javadoc added.
New functionality has a user manual doc added.
New PPL command checklist all confirmed.
API changes companion pull request created.
Commits are signed per the DCO using --signoff or -s.
Public documentation issue/PR created.

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.

github-actions · 2026-05-06T02:36:11Z

PR Reviewer Guide 🔍

(Review updated until commit `af2b288`)

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
📝 TODO sections api/src/main/java/org/opensearch/sql/api/UnifiedQueryPlanner.java [55]: Generate optimal physical plan to fully unify query execution and leverage Calcite's optimizer.
🔀 Multiple PR themes Sub-PR theme: Add postAnalysisRules extension point and OpenSearch datetime precision Relevant files: core/src/main/java/org/opensearch/sql/executor/OpenSearchTypeSystem.java api/src/main/java/org/opensearch/sql/api/spec/LanguageSpec.java Sub-PR theme: Implement DatetimeExtension with UDT normalize and output cast rules Relevant files: api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeExtension.java api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeUdtNormalizeRule.java api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeOutputCastRule.java api/src/main/java/org/opensearch/sql/api/spec/UnifiedPplSpec.java api/src/main/java/org/opensearch/sql/api/UnifiedQueryPlanner.java api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionTest.java
⚡ Recommended focus areas for review Stateful Singleton `DatetimeOutputCastRule.INSTANCE` is a singleton but `RelHomogeneousShuttle` may carry mutable traversal state. If the shuttle accumulates state across multiple `visit` calls (e.g., parent/child tracking), concurrent or sequential reuse of the singleton could produce incorrect results. Verify that `RelHomogeneousShuttle` is truly stateless before using a shared singleton. static final DatetimeOutputCastRule INSTANCE = new DatetimeOutputCastRule(); Stateful Singleton Same concern as `DatetimeOutputCastRule`: `DatetimeUdtNormalizeRule.INSTANCE` is a shared singleton of `RelHomogeneousShuttle`. If the base class holds any mutable traversal state, reusing the same instance across multiple `plan()` calls could cause subtle bugs. Confirm thread-safety and statelessness. static final DatetimeUdtNormalizeRule INSTANCE = new DatetimeUdtNormalizeRule(); Missing Recursion `visit(RelNode other)` does not call `super.visit(other)` before processing, so child nodes are never visited. The cast wrapping is only applied to the root node passed in. If the plan has nested projections with datetime fields that are not at the root, they will not be cast. Compare with `DatetimeUdtNormalizeRule` which correctly calls `super.visit(other)` first. public RelNode visit(RelNode other) { List<RelDataTypeField> fields = other.getRowType().getFieldList(); if (fields.stream().noneMatch(f -> isDatetimeType(f.getType().getSqlTypeName()))) { return other; } RexBuilder rexBuilder = other.getCluster().getRexBuilder(); List<RexNode> projects = new ArrayList<>(fields.size()); List<String> names = new ArrayList<>(fields.size()); // Cast datetime fields to VARCHAR for output; pass through others unchanged for (RelDataTypeField field : fields) { RexNode newField = rexBuilder.makeInputRef(other, field.getIndex()); RelDataType fieldType = field.getType(); if (isDatetimeType(fieldType.getSqlTypeName())) { projects.add(castToVarchar(rexBuilder, newField, fieldType)); } else { projects.add(newField); } names.add(field.getName()); } return LogicalProject.create(other, List.of(), projects, names); } UDT Nullable Mismatch `UdtMapping.fromUdtType` checks `instanceof AbstractExprRelDataType<?>` but does not handle the case where the UDT type is wrapped in a nullable type decorator. Depending on how Calcite wraps nullable types, the instanceof check may fail for nullable UDT columns, silently skipping normalization. static Optional<UdtMapping> fromUdtType(RelDataType type) { if (!(type instanceof AbstractExprRelDataType<?> e)) { return Optional.empty(); } ExprUDT udt = e.getUdt(); return Arrays.stream(values()).filter(u -> u.udtType == udt).findFirst(); } Rule Order Dependency Post-analysis rules are applied sequentially in registration order. `DatetimeUdtNormalizeRule` must run before `DatetimeOutputCastRule` because the cast rule checks for standard `SqlTypeName` datetime types (not UDTs). If the order is ever changed or a new extension registers rules in between, the output cast rule may miss UDT-typed columns. This ordering dependency is implicit and not enforced. for (var shuttle : context.getLangSpec().postAnalysisRules()) { plan = plan.accept(shuttle); } return plan;

github-actions · 2026-05-06T02:36:40Z

PR Code Suggestions ✨

Latest suggestions up to 58b0420

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Prevent casting intermediate nodes' datetime fields The rule visits every `RelNode` in the tree but only wraps the root — it should only apply to the root node. As written, it will wrap datetime fields in intermediate nodes (e.g., a `LogicalFilter` or inner `LogicalProject`) with a `CAST(...):VARCHAR`, which would break subsequent operations that expect the original datetime type. The rule should first recurse into children (via `super.visit(other)`) and then only apply the cast projection at the top-level call site, or guard against wrapping non-root nodes. api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeOutputCastRule.java [31-53] @Override public RelNode visit(RelNode other) { + // Do NOT recurse — this rule is intended to wrap only the root node List<RelDataTypeField> fields = other.getRowType().getFieldList(); if (fields.stream().noneMatch(f -> isDatetimeType(f.getType().getSqlTypeName()))) { return other; } - ... + + RexBuilder rexBuilder = other.getCluster().getRexBuilder(); + List<RexNode> projects = new ArrayList<>(fields.size()); + List<String> names = new ArrayList<>(fields.size()); + + for (RelDataTypeField field : fields) { + RexNode newField = rexBuilder.makeInputRef(other, field.getIndex()); + RelDataType fieldType = field.getType(); + if (isDatetimeType(fieldType.getSqlTypeName())) { + projects.add(castToVarchar(rexBuilder, newField, fieldType)); + } else { + projects.add(newField); + } + names.add(field.getName()); + } return LogicalProject.create(other, List.of(), projects, names); } Suggestion importance[1-10]: 6 __ Why: The concern about wrapping intermediate nodes is valid in principle, but looking at the test cases (e.g., `testAllStandardDatetimeTypesCastToVarchar`), the rule appears to intentionally wrap all nodes with datetime fields. The `improved_code` is essentially the same as the existing code minus the `super.visit()` call, but the suggestion doesn't clearly demonstrate the fix. The behavior concern is real but the proposed fix may break the intended recursive traversal.	Low
Possible issue	Avoid reusing stateful shuttle singletons across plans `RelShuttle` implementations like `RelHomogeneousShuttle` are stateful — they may cache visited nodes internally. Reusing the same `INSTANCE` singleton across multiple calls to `plan()` can cause incorrect results or stale state being applied to a new plan. Each rule should be instantiated fresh per planning call rather than using shared singletons. api/src/main/java/org/opensearch/sql/api/UnifiedQueryPlanner.java [66-70] +RelNode plan = strategy.plan(query); +for (var shuttle : context.getLangSpec().postAnalysisRules()) { + plan = plan.accept(shuttle); +} +return plan; - Suggestion importance[1-10]: 5 __ Why: The concern about stateful `RelShuttle` singletons is valid — `RelHomogeneousShuttle` uses a `visited` set internally that could cause issues across multiple planning calls. However, the `improved_code` is identical to the `existing_code`, making the suggestion incomplete as it doesn't show the actual fix (instantiating fresh shuttles per call).	Low
General	Assert all matching calls have correct type The helper only captures the first matching `RexCall` found (`ref.get() == null`), but after `DatetimeUdtNormalizeRule` rewrites the plan, the same operator name may appear multiple times (e.g., `DATE` appears twice in `testNestedUdfCallsNormalized`). The test for `DATE` in that case will silently validate only the first occurrence, potentially missing a non-normalized call. Consider asserting that all matching calls have the expected type, not just the first. api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionTest.java [196-224] private static void assertCallType( RelNode plan, String operatorName, SqlTypeName expectedType, int expectedPrecision) { - AtomicReference<RexCall> ref = new AtomicReference<>(); + List<RexCall> found = new ArrayList<>(); plan.accept( new RelHomogeneousShuttle() { @Override public RelNode visit(RelNode other) { RelNode visited = super.visit(other); visited.accept( new RexShuttle() { @Override public RexNode visitCall(RexCall call) { - if (ref.get() == null - && call.getOperator().getName().equalsIgnoreCase(operatorName)) { - ref.set(call); + if (call.getOperator().getName().equalsIgnoreCase(operatorName)) { + found.add(call); } return super.visitCall(call); } }); return visited; } }); + assertFalse("No RexCall found for: " + operatorName, found.isEmpty()); + for (RexCall call : found) { + assertEquals(operatorName + " type", expectedType, call.getType().getSqlTypeName()); + if (expectedPrecision >= 0) { + assertEquals(operatorName + " precision", expectedPrecision, call.getType().getPrecision()); + } + } +} Suggestion importance[1-10]: 4 __ Why: The suggestion to validate all matching `RexCall` instances rather than just the first is a reasonable improvement to test thoroughness. However, this is a test quality improvement with limited impact on correctness of the production code.	Low

Previous suggestions

Suggestions up to commit af2b288

Category Suggestion Impact

Possible issue

Recursively visit child nodes before applying cast rule

The visit method does not recursively visit child nodes before applying the cast
rule. This means datetime fields in non-root nodes (e.g., sub-queries or nested
projections) will not be processed. The method should call super.visit(other) or
visitChildren(other) first to ensure the entire tree is traversed.

api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeOutputCastRule.java [31-53]

 @Override
 public RelNode visit(RelNode other) {
-  List<RelDataTypeField> fields = other.getRowType().getFieldList();
+  RelNode visited = visitChildren(other);
+  List<RelDataTypeField> fields = visited.getRowType().getFieldList();
   if (fields.stream().noneMatch(f -> isDatetimeType(f.getType().getSqlTypeName()))) {
-    return other;
+    return visited;
   }
-  ...
-  return LogicalProject.create(other, List.of(), projects, names);
+
+  RexBuilder rexBuilder = visited.getCluster().getRexBuilder();
+  List<RexNode> projects = new ArrayList<>(fields.size());
+  List<String> names = new ArrayList<>(fields.size());
+
+  for (RelDataTypeField field : fields) {
+    RexNode newField = rexBuilder.makeInputRef(visited, field.getIndex());
+    RelDataType fieldType = field.getType();
+    if (isDatetimeType(fieldType.getSqlTypeName())) {
+      projects.add(castToVarchar(rexBuilder, newField, fieldType));
+    } else {
+      projects.add(newField);
+    }
+    names.add(field.getName());
+  }
+  return LogicalProject.create(visited, List.of(), projects, names);
 }

Suggestion importance[1-10]: 6

__

Why: The DatetimeOutputCastRule is designed as an output cast rule (applied to the root/top-level node), so not recursing into children may be intentional. However, if the rule is meant to wrap all datetime fields at every level, the missing visitChildren call could cause incomplete processing. The suggestion is valid but the intent of the rule (root-only vs. full-tree) is ambiguous from context.

Low

General

Assert all matching calls match expected type

The assertCallType helper only finds the first matching RexCall by operator name,
but multiple calls with the same operator name may exist in the plan (e.g., two
DATE(...) calls). If the first found call does not match the expected
type/precision, the assertion will fail misleadingly. Consider collecting all
matching calls and asserting that at least one (or all) match the expected type.

api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionTest.java [196-224]

 private static void assertCallType(
     RelNode plan, String operatorName, SqlTypeName expectedType, int expectedPrecision) {
-  AtomicReference<RexCall> ref = new AtomicReference<>();
+  List<RexCall> found = new ArrayList<>();
   plan.accept(
       new RelHomogeneousShuttle() {
         @Override
         public RelNode visit(RelNode other) {
           RelNode visited = super.visit(other);
           visited.accept(
               new RexShuttle() {
                 @Override
                 public RexNode visitCall(RexCall call) {
-                  if (ref.get() == null
-                      && call.getOperator().getName().equalsIgnoreCase(operatorName)) {
-                    ref.set(call);
+                  if (call.getOperator().getName().equalsIgnoreCase(operatorName)) {
+                    found.add(call);
                   }
                   return super.visitCall(call);
                 }
               });
           return visited;
         }
       });
+  assertFalse("No RexCall found for: " + operatorName, found.isEmpty());
+  for (RexCall call : found) {
+    assertEquals(operatorName + " type", expectedType, call.getType().getSqlTypeName());
+    if (expectedPrecision >= 0) {
+      assertEquals(operatorName + " precision", expectedPrecision, call.getType().getPrecision());
+    }
+  }
+}

Suggestion importance[1-10]: 4

__

Why: The suggestion improves test robustness by checking all matching RexCall nodes rather than just the first one, which could catch cases where some calls have incorrect types. However, this is a test-quality improvement with limited impact on correctness of the production code.

Low

Suggestions up to commit cc90d7f

Category	Suggestion	Impact
Possible issue	Prevent recursive application of output cast rule The rule visits every `RelNode` in the tree but only wraps the root — it should only apply to the top-level node, not recursively to all nodes. As written, `visit(RelNode` `other)` is called for every node in the plan tree (because `RelHomogeneousShuttle` recurses via `super.visit`), so inner nodes that happen to have datetime fields will also get a cast projection injected, corrupting the plan. The rule should not call `super.visit(other)` to recurse into children, or it should be applied only to the root node externally. api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeOutputCastRule.java [31-53] @Override public RelNode visit(RelNode other) { + // Do NOT recurse into children; this rule is intended only for the root output node. List<RelDataTypeField> fields = other.getRowType().getFieldList(); if (fields.stream().noneMatch(f -> isDatetimeType(f.getType().getSqlTypeName()))) { return other; } - ... + + RexBuilder rexBuilder = other.getCluster().getRexBuilder(); + List<RexNode> projects = new ArrayList<>(fields.size()); + List<String> names = new ArrayList<>(fields.size()); + + for (RelDataTypeField field : fields) { + RexNode newField = rexBuilder.makeInputRef(other, field.getIndex()); + RelDataType fieldType = field.getType(); + if (isDatetimeType(fieldType.getSqlTypeName())) { + projects.add(castToVarchar(rexBuilder, newField, fieldType)); + } else { + projects.add(newField); + } + names.add(field.getName()); + } return LogicalProject.create(other, List.of(), projects, names); } Suggestion importance[1-10]: 7 __ Why: The `DatetimeOutputCastRule` extends `RelHomogeneousShuttle` which recurses into children by default via `super.visit()`. Since the rule doesn't call `super.visit(other)`, it actually doesn't recurse — but the suggestion raises a valid concern about intent. However, looking at the code, `visit(RelNode other)` does NOT call `super.visit(other)`, so it won't recurse into children. The suggestion misreads the code but the underlying concern about applying casts to inner nodes is worth noting if the implementation were to change.	Medium
Possible issue	Avoid shared mutable shuttle singleton for thread safety `RelHomogeneousShuttle` (and `RelShuttle` implementations in general) are not thread-safe because they maintain mutable traversal state. Sharing a single `INSTANCE` across concurrent query planning calls can lead to race conditions. Each call to `postAnalysisRules()` should return a new instance instead of a shared singleton. api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeUdtNormalizeRule.java [29] -static final DatetimeUdtNormalizeRule INSTANCE = new DatetimeUdtNormalizeRule(); +// Remove INSTANCE singleton; instantiate per use in DatetimeExtension.postAnalysisRules() +// In DatetimeExtension: +// return List.of(new DatetimeUdtNormalizeRule(), new DatetimeOutputCastRule()); Suggestion importance[1-10]: 6 __ Why: The `INSTANCE` singleton pattern for `RelHomogeneousShuttle` subclasses can be problematic in concurrent environments since shuttles may maintain traversal state. The suggestion to instantiate per use is valid for thread safety, though the actual risk depends on whether the shuttle maintains mutable state during traversal.	Low
General	Guard against null plan after shuttle application `plan.accept(shuttle)` returns a `RelNode` but `RelShuttle.accept` may return the same node or a new one. If a shuttle returns `null` (e.g., due to an unhandled node type), the next iteration will throw a `NullPointerException`. Add a null-check or assert after each shuttle application to fail fast with a meaningful error. api/src/main/java/org/opensearch/sql/api/UnifiedQueryPlanner.java [66-70] RelNode plan = strategy.plan(query); for (var shuttle : context.getLangSpec().postAnalysisRules()) { - plan = plan.accept(shuttle); + RelNode result = plan.accept(shuttle); + if (result == null) { + throw new IllegalStateException( + "Post-analysis rule " + shuttle.getClass().getSimpleName() + " returned null plan"); + } + plan = result; } return plan; Suggestion importance[1-10]: 3 __ Why: While adding a null-check is a defensive practice, `RelShuttle` implementations in Calcite are not expected to return `null` — they always return a `RelNode`. This is a low-impact defensive check that adds verbosity without addressing a realistic failure scenario.	Low

Suggestions up to commit cc90d7f

Category	Suggestion	Impact
Possible issue	Return new rule instances to avoid shared state Both `DatetimeUdtNormalizeRule` and `DatetimeOutputCastRule` extend `RelHomogeneousShuttle`, which is stateful (it tracks visited nodes). Returning shared `INSTANCE` singletons means concurrent or sequential queries will share and corrupt that state. Return new instances on each call instead. api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeExtension.java [23-26] @Override public List<RelShuttle> postAnalysisRules() { - return List.of(DatetimeUdtNormalizeRule.INSTANCE, DatetimeOutputCastRule.INSTANCE); + return List.of(new DatetimeUdtNormalizeRule(), new DatetimeOutputCastRule()); } Suggestion importance[1-10]: 7 __ Why: `RelHomogeneousShuttle` maintains a visited-node set as internal state, making shared singletons unsafe for concurrent or even sequential use. Returning new instances per call is a correct and important fix, and the `improved_code` accurately reflects the change needed.	Medium
	Prevent recursive application of output cast rule The rule visits every `RelNode` in the tree but only wraps the root — it should only apply to the top-level node. As written, it will also wrap intermediate nodes (e.g., inner `LogicalProject`s), causing redundant or incorrect double-casting of datetime fields. The rule should first recurse into children and then only apply the cast projection to the root, or it should be applied only once on the root outside the shuttle. api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeOutputCastRule.java [31-53] @Override public RelNode visit(RelNode other) { + // Do NOT recurse into children; this rule is intended for the root node only. List<RelDataTypeField> fields = other.getRowType().getFieldList(); if (fields.stream().noneMatch(f -> isDatetimeType(f.getType().getSqlTypeName()))) { return other; } - ... + + RexBuilder rexBuilder = other.getCluster().getRexBuilder(); + List<RexNode> projects = new ArrayList<>(fields.size()); + List<String> names = new ArrayList<>(fields.size()); + + for (RelDataTypeField field : fields) { + RexNode newField = rexBuilder.makeInputRef(other, field.getIndex()); + RelDataType fieldType = field.getType(); + if (isDatetimeType(fieldType.getSqlTypeName())) { + projects.add(castToVarchar(rexBuilder, newField, fieldType)); + } else { + projects.add(newField); + } + names.add(field.getName()); + } return LogicalProject.create(other, List.of(), projects, names); } Suggestion importance[1-10]: 5 __ Why: The concern about recursive application is valid in principle, but looking at the test cases (e.g., `testAllStandardDatetimeTypesCastToVarchar`), the expected plan shows the cast only on the outer `LogicalProject`, suggesting the current behavior may be intentional or the tests validate the actual behavior. The `improved_code` removes the call to `super.visit()` which would break child traversal needed by `DatetimeUdtNormalizeRule`. The suggestion has merit but the improved code doesn't accurately reflect a safe fix.	Low
	Avoid shared mutable shuttle singleton for thread safety `RelHomogeneousShuttle` (and `RelShuttle` in general) is not thread-safe because it maintains internal state (visited set). Sharing a single `INSTANCE` across concurrent queries will cause race conditions. Each call to `postAnalysisRules()` should return a new instance instead of a shared singleton. api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeUdtNormalizeRule.java [29] -static final DatetimeUdtNormalizeRule INSTANCE = new DatetimeUdtNormalizeRule(); +// Remove INSTANCE singleton; instantiate per use in DatetimeExtension.postAnalysisRules() +// In DatetimeExtension: +// return List.of(new DatetimeUdtNormalizeRule(), new DatetimeOutputCastRule()); Suggestion importance[1-10]: 3 __ Why: The thread-safety concern about `RelHomogeneousShuttle` is valid, but the `improved_code` is just a comment rather than actual code, making it an incomplete suggestion. The actual fix should be in `DatetimeExtension`, which is covered by suggestion 3.	Low

Suggestions up to commit 01fba06

Category	Suggestion	Impact
Possible issue	Recursively visit children before applying cast rule The rule does not recursively visit child nodes before applying the cast. If a datetime field appears in a nested node (not the root), it will be missed. Call `super.visit(other)` first to ensure children are visited before processing the current node. api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeOutputCastRule.java [31-36] @Override public RelNode visit(RelNode other) { - List<RelDataTypeField> fields = other.getRowType().getFieldList(); + RelNode visited = super.visit(other); + List<RelDataTypeField> fields = visited.getRowType().getFieldList(); if (fields.stream().noneMatch(f -> isDatetimeType(f.getType().getSqlTypeName()))) { - return other; + return visited; } + RexBuilder rexBuilder = visited.getCluster().getRexBuilder(); + List<RexNode> projects = new ArrayList<>(fields.size()); + List<String> names = new ArrayList<>(fields.size()); + for (RelDataTypeField field : fields) { + RexNode newField = rexBuilder.makeInputRef(visited, field.getIndex()); + RelDataType fieldType = field.getType(); + if (isDatetimeType(fieldType.getSqlTypeName())) { + projects.add(castToVarchar(rexBuilder, newField, fieldType)); + } else { + projects.add(newField); + } + names.add(field.getName()); + } + return LogicalProject.create(visited, List.of(), projects, names); +} Suggestion importance[1-10]: 6 __ Why: The `DatetimeOutputCastRule` doesn't call `super.visit(other)` before processing, meaning it only applies to the root node and misses datetime fields in nested nodes. The `DatetimeUdtNormalizeRule` correctly calls `super.visit(other)` first, so this inconsistency could be a real issue depending on the intended behavior. However, since this rule is described as wrapping "the root output", it may be intentional to only apply at the top level.	Low
General	Close ResultSet to prevent resource leaks The `ResultSet` is not closed after use, which can cause resource leaks. Wrap it in a try-with-resources block to ensure it is always closed. api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionTest.java [167-174] -ResultSet resultSet = statement.executeQuery(); -verify(resultSet) - .expectSchema( - col("hire_date", VARCHAR), col("start_time", VARCHAR), col("created_at", VARCHAR)) - .expectData( - row("2024-01-16", "12:00:00", "2024-01-15 08:00:00"), - row("2024-06-20", "14:00:00", "2024-06-20 00:00:00")); +try (ResultSet resultSet = statement.executeQuery()) { + verify(resultSet) + .expectSchema( + col("hire_date", VARCHAR), col("start_time", VARCHAR), col("created_at", VARCHAR)) + .expectData( + row("2024-01-16", "12:00:00", "2024-01-15 08:00:00"), + row("2024-06-20", "14:00:00", "2024-06-20 00:00:00")); +} Suggestion importance[1-10]: 4 __ Why: The `ResultSet` is not closed after use, which can cause resource leaks. Wrapping it in a try-with-resources block is a valid improvement, though the impact is lower in test code since resources are typically cleaned up when the test JVM exits.	Low
General	Avoid shared mutable shuttle instances across queries `RelShuttle.accept` returns a `RelNode`, but `RelHomogeneousShuttle` implementations may return the same instance if no changes are made. However, the shuttles used here (`DatetimeUdtNormalizeRule`, `DatetimeOutputCastRule`) are stateful singletons shared across calls. If these shuttles accumulate any internal state between invocations, concurrent or sequential queries could interfere. Consider instantiating new shuttle instances per query rather than using shared `INSTANCE` singletons. api/src/main/java/org/opensearch/sql/api/UnifiedQueryPlanner.java [66-70] +RelNode plan = strategy.plan(query); +for (var shuttle : context.getLangSpec().postAnalysisRules()) { + plan = plan.accept(shuttle); +} +return plan; - Suggestion importance[1-10]: 3 __ Why: The suggestion raises a valid concern about singleton shuttle instances, but `RelHomogeneousShuttle` is typically stateless and the `improved_code` is identical to the `existing_code`, making this more of a verification suggestion than an actionable fix.	Low

Suggestions up to commit 70aabfe

Category	Suggestion	Impact
Possible issue	Missing recursive traversal of child nodes The `visit` method does not call `super.visit(other)` before processing the node, meaning child nodes are never recursively visited. This causes the rule to only apply to the root node and miss datetime fields in nested `RelNode`s. Call `super.visit(other)` first and operate on the returned node, consistent with how `DatetimeUdtNormalizeRule` handles this. api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeOutputCastRule.java [28-32] public RelNode visit(RelNode other) { - List<RelDataTypeField> fields = other.getRowType().getFieldList(); + RelNode visited = super.visit(other); + List<RelDataTypeField> fields = visited.getRowType().getFieldList(); if (fields.stream().noneMatch(f -> isDatetimeType(f.getType().getSqlTypeName()))) { - return other; + return visited; } Suggestion importance[1-10]: 7 __ Why: The `DatetimeOutputCastRule.visit` method doesn't call `super.visit(other)` unlike `DatetimeUdtNormalizeRule`, meaning child nodes are never recursively visited. However, the rule is designed to wrap the root output with CAST for wire-format compatibility, so it may be intentionally applied only at the top level. The inconsistency with `DatetimeUdtNormalizeRule` is worth noting, but the impact depends on the intended behavior.	Medium
General	ResultSet resource not closed after use The `ResultSet` is never closed after use, which can leak JDBC resources. Wrap `resultSet` in a try-with-resources block or close it in a `finally` block to ensure proper cleanup. api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionTest.java [167-174] -ResultSet resultSet = statement.executeQuery(); -verify(resultSet) - .expectSchema( - col("hire_date", VARCHAR), col("start_time", VARCHAR), col("created_at", VARCHAR)) - .expectData( - row("2024-01-16", "12:00:00", "2024-01-15 08:00:00"), - row("2024-06-20", "14:00:00", "2024-06-20 00:00:00")); +try (ResultSet resultSet = statement.executeQuery()) { + verify(resultSet) + .expectSchema( + col("hire_date", VARCHAR), col("start_time", VARCHAR), col("created_at", VARCHAR)) + .expectData( + row("2024-01-16", "12:00:00", "2024-01-15 08:00:00"), + row("2024-06-20", "14:00:00", "2024-06-20 00:00:00")); +} Suggestion importance[1-10]: 5 __ Why: The `ResultSet` is not closed after use, which can leak JDBC resources. The suggested fix of wrapping in try-with-resources is correct and the `improved_code` accurately reflects the change, though this is a test file so the impact is limited.	Low
General	Shared shuttle singletons risk concurrency issues `RelShuttle.accept` returns a `RelNode`, but `RelHomogeneousShuttle` instances are stateful — reusing the same `INSTANCE` singleton across multiple concurrent queries could cause race conditions if the shuttle holds any mutable state. Consider instantiating a new shuttle per invocation rather than using shared singletons (`DatetimeUdtNormalizeRule.INSTANCE`, `DatetimeOutputCastRule.INSTANCE`), or ensure the shuttles are truly stateless. api/src/main/java/org/opensearch/sql/api/UnifiedQueryPlanner.java [66-70] +RelNode plan = strategy.plan(query); +for (var shuttle : context.getLangSpec().postAnalysisRules()) { + plan = plan.accept(shuttle); +} +return plan; - Suggestion importance[1-10]: 4 __ Why: The concern about singleton `INSTANCE` shuttles being stateful is valid in principle, but the `improved_code` is identical to `existing_code`, making this more of a verification suggestion. The actual fix would need to be in `DatetimeExtension.postAnalysisRules()` to return new instances, not in `UnifiedQueryPlanner`.	Low

github-actions · 2026-05-06T18:33:48Z

Persistent review updated to latest commit 01fba06

dai-chen · 2026-05-06T18:42:22Z

All test related CI is failling on Gradle version issue:

* What went wrong:
A problem occurred evaluating root project 'opensearch-sql'.
> Failed to apply plugin class 'org.opensearch.gradle.info.GlobalBuildInfoPlugin'.
   > Gradle 9.4.1+ is required

github-actions · 2026-05-07T01:55:17Z

Persistent review updated to latest commit cc90d7f

dai-chen · 2026-05-07T02:07:43Z

All test related CI is failling on Gradle version issue:

* What went wrong:
A problem occurred evaluating root project 'opensearch-sql'.
> Failed to apply plugin class 'org.opensearch.gradle.info.GlobalBuildInfoPlugin'.
   > Gradle 9.4.1+ is required

Will rebase once #5414 merged.

github-actions · 2026-05-07T02:55:27Z

Persistent review updated to latest commit af2b288

Add postAnalysisRules (List<RelShuttle>) to LanguageSpec.LanguageExtension and register DatetimeExtension in UnifiedPplSpec with two rules: 1. DatetimeUdtNormalizeRule rewrites datetime UDT return types (EXPR_DATE/TIME/TIMESTAMP) on RexCall nodes to standard Calcite DATE/TIME(9)/TIMESTAMP(9) types via call.clone(). Precision is derived from the type system (OpenSearchTypeSystem.getMaxPrecision). 2. DatetimeOutputCastRule adds a final LogicalProject that casts standard datetime output columns to VARCHAR, aligning with PPL's wire-format contract (ISO string representation). Both rules run as postAnalysisRules after the planning strategy produces the RelNode, applied uniformly to both SQL and PPL paths. Also bumps OpenSearchTypeSystem max datetime precision from 3 to 9 (nanosecond) for TIME and TIMESTAMP types. No changes to UDF definitions or implementors in core/ — the mismatch between rewritten signatures and UDF implementations is a known limitation addressed separately. Signed-off-by: Chen Dai <daichen@amazon.com>

@ignore

Bring the analytics-engine catch-up PR up to current upstream/main by resolving conflicts introduced by 4 main commits since 2026-04-30: - opensearch-project#5419 (LENGTH/REGEXP_REPLACE/DATE_TRUNC unified function spec) - opensearch-project#5408 (datetime type normalization) - opensearch-project#5414 (Gradle wrapper bump + @ignore exclusion) - opensearch-project#5399 (FGAC-scoped SQL cursor continuation) Resolutions: api/spec/* (LanguageSpec, UnifiedFunctionSpec, UnifiedPplSpec, UnifiedSqlSpec): took main. Main is a strict superset — adds postAnalysisRules and preCompilationRules extension points, the new FunctionSpecBuilder DSL, SCALAR category for length/regexp_replace/ date_trunc, the DatetimeExtension on PPL spec, and the CoreExtension wiring on SQL spec. PR's RELEVANCE category is preserved unchanged. api/UnifiedQueryPlanner.java, api/compiler/UnifiedQueryCompiler.java: took main. Both adopt the new postAnalysisRules / preCompilationRules hooks introduced in opensearch-project#5408 / opensearch-project#5419. core/executor/QueryService.java: composed both sides — kept HEAD's CalciteClassLoaderHelper.withCalciteClassLoader wrapper around main's StageErrorHandler stage tracking. Same pattern as the original PR resolution; both improvements are orthogonal. plugin/SQLPlugin.java: kept HEAD imports for ExplainResponse and QueryType (referenced by createSqlAnalyticsRouter, which only exists on the feature branch). plugin/transport/TransportPPLQueryAction.java: kept HEAD's queryPlanExecutor parameter alongside main's extensionsHolder parameter, since both are referenced in the constructor body. legacy/plugin/RestSqlAction.java: took HEAD. The 3-way merge produced a duplicated handleException/getRawErrorCode block; HEAD already contained both the delegateToV2Engine refactor and the ErrorReport unwrap from main, so HEAD is the correct superset. Build: :api, :core, :opensearch-sql-plugin, :legacy compileJava and unit tests pass; spotlessCheck passes. Signed-off-by: Kai Huang <ahkcs@amazon.com>

@ignore

…ation Brings the catch-up branch up to current upstream/main (4 commits since this PR was opened) and current feature/mustang-ppl-integration (9 commits since this PR was opened), so the PR is mergeable into feature/mustang-ppl-integration without conflicts. Squashed (rather than two real merge commits) for the same DCO reason the original commit was squashed: upstream commits authored by many contributors with inconsistent or missing Signed-off-by trailers would otherwise be brought into this PR's history. Newer main commits absorbed (4): - opensearch-project#5419 (LENGTH/REGEXP_REPLACE/DATE_TRUNC unified function spec) - opensearch-project#5408 (datetime type normalization) - opensearch-project#5414 (Gradle wrapper bump + @ignore exclusion) - opensearch-project#5399 (FGAC-scoped SQL cursor continuation) Newer feature commits absorbed (9): - opensearch-project#5403 (analytics-engine optional dependency — major rewiring) - opensearch-project#5407 (Carry CalciteEvalCommandIT through helper-managed index path) - opensearch-project#5413 (Default plugins.calcite.enabled=true on unified path) - opensearch-project#5415, opensearch-project#5416, opensearch-project#5417, opensearch-project#5409, opensearch-project#5400, opensearch-project#5406 (smaller carryovers + bumps) Conflict resolutions (10 from main side, 3 from feature side): api/spec/* (LanguageSpec, UnifiedFunctionSpec, UnifiedPplSpec, UnifiedSqlSpec): took main. Main is a strict superset — adds postAnalysisRules and preCompilationRules extension points, the new FunctionSpecBuilder DSL, SCALAR category for length/regexp_replace/ date_trunc, the DatetimeExtension on PPL spec, and the CoreExtension wiring on SQL spec. PR's RELEVANCE category is preserved unchanged. api/UnifiedQueryPlanner.java, api/compiler/UnifiedQueryCompiler.java: took main. Both adopt the new postAnalysisRules / preCompilationRules hooks introduced in opensearch-project#5408 / opensearch-project#5419. core/executor/QueryService.java: composed both sides — kept HEAD's CalciteClassLoaderHelper.withCalciteClassLoader wrapper around main's StageErrorHandler stage tracking. Same pattern as the original PR resolution; both improvements are orthogonal. legacy/plugin/RestSqlAction.java: took HEAD. The 3-way merge produced a duplicated handleException/getRawErrorCode block; HEAD already contained both the delegateToV2Engine refactor and the ErrorReport unwrap from main, so HEAD is the correct superset. integ-test/build.gradle: took feature. Both sides added the same @ignore exclusion block; feature has alphabetical ordering and a more detailed comment explaining the Gradle 9.4.1 TestEventReporterAsListener cast bug. integ-test/.../CalciteEvalCommandIT.java: composed both sides. Took feature's helper-managed test_eval provisioning (createIndexByRestClient + isIndexExist guard, from opensearch-project#5407) so analytics-engine compatibility runs get a parquet-backed index. Added back PR HEAD's test_eval_agent setup (needed by the dotted-path eval tests for opensearch-project#5351) wrapped in its own isIndexExist guard for the same parquet-aware idempotency. plugin/.../TransportPPLQueryAction.java: took feature. PR opensearch-project#5403 made analytics-engine an optional dependency by moving QueryPlanExecutor from a required constructor parameter to an @Inject(optional=true) setter. Feature's design supersedes our prior wiring. plugin/.../SQLPlugin.java: took feature. The same opensearch-project#5403 simplification removed loadExtensions/EngineExtensionsHolder/executionEngineExtensions plumbing (no longer needed once analytics-engine is optionally bound). Feature retains the createSqlAnalyticsRouter method this PR introduced. plugin/.../config/EngineExtensionsHolder.java: deleted. Unreferenced after taking feature's SQLPlugin/TransportPPLQueryAction; not present on feature branch. Build: :api, :core, :opensearch-sql-plugin, :legacy compileJava + :integ-test compileTestJava all pass; unit tests pass; spotlessCheck clean. Signed-off-by: Kai Huang <ahkcs@amazon.com>

@ignore

Single squashed commit on top of feature/mustang-ppl-integration that absorbs upstream/main's commits not yet on the feature branch. Replaces the prior catch-up squash (opensearch-project#5396 base + the original af831d3 rebase commit) so this PR is a fast-forward into feature/mustang-ppl-integration. Squashed (rather than a merge commit) because upstream main commits were authored by many contributors with inconsistent or missing Signed-off-by trailers; DCO would otherwise reject those commits. Main commits absorbed (54 since divergence; 4 since the original catch-up squash was made on 2026-04-30): - opensearch-project#5419 (LENGTH/REGEXP_REPLACE/DATE_TRUNC unified function spec) - opensearch-project#5408 (datetime type normalization) - opensearch-project#5414 (Gradle wrapper bump + @ignore exclusion) - opensearch-project#5399 (FGAC-scoped SQL cursor continuation) - opensearch-project#5394 (SQL Vector Search), opensearch-project#5361 (OpenSearch 3.7), opensearch-project#5360 (unified SQL language spec), opensearch-project#5240 (PPL Union), and 46 others. Conflict resolutions: api/spec/* (LanguageSpec, UnifiedFunctionSpec, UnifiedPplSpec, UnifiedSqlSpec): took main. Main is a strict superset — adds postAnalysisRules and preCompilationRules extension points, the new FunctionSpecBuilder DSL, SCALAR category for length/regexp_replace/ date_trunc, the DatetimeExtension on PPL spec, and the CoreExtension wiring on SQL spec. RELEVANCE category is preserved unchanged. api/UnifiedQueryPlanner.java, api/compiler/UnifiedQueryCompiler.java: took main. Both adopt the new postAnalysisRules / preCompilationRules hooks introduced in opensearch-project#5408 / opensearch-project#5419. core/executor/QueryService.java: composed both sides — kept feature's CalciteClassLoaderHelper.withCalciteClassLoader wrapper around main's StageErrorHandler stage tracking; both improvements are orthogonal. legacy/plugin/RestSqlAction.java: took feature. The 3-way merge produced a duplicated handleException/getRawErrorCode block; feature already contained both the delegateToV2Engine refactor and the ErrorReport unwrap from main, so feature is the correct superset. CLAUDE.md, docs/user/ppl/functions/condition.md: took main. explain_streamstats_global{,_null_bucket}.yaml: took main (post-opensearch-project#5359 shape). core/CalciteRelNodeVisitor + utils/PlanUtils: took main (collation utility hoisted from CalciteRelNodeVisitor.backtrackForCollation into PlanUtils.findInputCollation). integ-test/CalciteNoPushdownIT.java: added CalciteMixedFieldTypeIT. ppl/antlr/OpenSearchPPLParser.g4: added unionCommand. ppl/calcite/CalcitePPLStreamstatsTest.java: added testMultipleStreamstatsWithWindow. integ-test/build.gradle: took feature. Both sides added the same @ignore exclusion block; feature has alphabetical ordering and a more detailed comment explaining the Gradle 9.4.1 cast bug. integ-test/.../CalciteEvalCommandIT.java: composed both sides. Took feature's helper-managed test_eval provisioning (createIndexByRestClient + isIndexExist guard, from opensearch-project#5407) so analytics-engine compatibility runs get a parquet-backed index. Added the test_eval_agent setup (needed by the dotted-path eval tests for opensearch-project#5351) wrapped in its own isIndexExist guard for the same parquet-aware idempotency. plugin/.../TransportPPLQueryAction.java, plugin/.../SQLPlugin.java: took feature. PR opensearch-project#5403 made analytics-engine an optional dependency by moving QueryPlanExecutor from a required constructor parameter to an @Inject(optional=true) setter, and removed the loadExtensions / EngineExtensionsHolder / executionEngineExtensions plumbing. Feature retains the createSqlAnalyticsRouter method this catch-up introduced. plugin/.../config/EngineExtensionsHolder.java: deleted (unreferenced post-opensearch-project#5403; not present on feature). Build: :api, :core, :opensearch-sql-plugin, :legacy compileJava + :integ-test compileTestJava all pass; unit tests pass; spotlessCheck clean. Signed-off-by: Kai Huang <ahkcs@amazon.com>

@ignore

…5397) Single squashed commit on top of feature/mustang-ppl-integration that absorbs upstream/main's commits not yet on the feature branch. Replaces the prior catch-up squash (#5396 base + the original af831d3 rebase commit) so this PR is a fast-forward into feature/mustang-ppl-integration. Squashed (rather than a merge commit) because upstream main commits were authored by many contributors with inconsistent or missing Signed-off-by trailers; DCO would otherwise reject those commits. Main commits absorbed (54 since divergence; 4 since the original catch-up squash was made on 2026-04-30): - #5419 (LENGTH/REGEXP_REPLACE/DATE_TRUNC unified function spec) - #5408 (datetime type normalization) - #5414 (Gradle wrapper bump + @ignore exclusion) - #5399 (FGAC-scoped SQL cursor continuation) - #5394 (SQL Vector Search), #5361 (OpenSearch 3.7), #5360 (unified SQL language spec), #5240 (PPL Union), and 46 others. Conflict resolutions: api/spec/* (LanguageSpec, UnifiedFunctionSpec, UnifiedPplSpec, UnifiedSqlSpec): took main. Main is a strict superset — adds postAnalysisRules and preCompilationRules extension points, the new FunctionSpecBuilder DSL, SCALAR category for length/regexp_replace/ date_trunc, the DatetimeExtension on PPL spec, and the CoreExtension wiring on SQL spec. RELEVANCE category is preserved unchanged. api/UnifiedQueryPlanner.java, api/compiler/UnifiedQueryCompiler.java: took main. Both adopt the new postAnalysisRules / preCompilationRules hooks introduced in #5408 / #5419. core/executor/QueryService.java: composed both sides — kept feature's CalciteClassLoaderHelper.withCalciteClassLoader wrapper around main's StageErrorHandler stage tracking; both improvements are orthogonal. legacy/plugin/RestSqlAction.java: took feature. The 3-way merge produced a duplicated handleException/getRawErrorCode block; feature already contained both the delegateToV2Engine refactor and the ErrorReport unwrap from main, so feature is the correct superset. CLAUDE.md, docs/user/ppl/functions/condition.md: took main. explain_streamstats_global{,_null_bucket}.yaml: took main (post-#5359 shape). core/CalciteRelNodeVisitor + utils/PlanUtils: took main (collation utility hoisted from CalciteRelNodeVisitor.backtrackForCollation into PlanUtils.findInputCollation). integ-test/CalciteNoPushdownIT.java: added CalciteMixedFieldTypeIT. ppl/antlr/OpenSearchPPLParser.g4: added unionCommand. ppl/calcite/CalcitePPLStreamstatsTest.java: added testMultipleStreamstatsWithWindow. integ-test/build.gradle: took feature. Both sides added the same @ignore exclusion block; feature has alphabetical ordering and a more detailed comment explaining the Gradle 9.4.1 cast bug. integ-test/.../CalciteEvalCommandIT.java: composed both sides. Took feature's helper-managed test_eval provisioning (createIndexByRestClient + isIndexExist guard, from #5407) so analytics-engine compatibility runs get a parquet-backed index. Added the test_eval_agent setup (needed by the dotted-path eval tests for #5351) wrapped in its own isIndexExist guard for the same parquet-aware idempotency. plugin/.../TransportPPLQueryAction.java, plugin/.../SQLPlugin.java: took feature. PR #5403 made analytics-engine an optional dependency by moving QueryPlanExecutor from a required constructor parameter to an @Inject(optional=true) setter, and removed the loadExtensions / EngineExtensionsHolder / executionEngineExtensions plumbing. Feature retains the createSqlAnalyticsRouter method this catch-up introduced. plugin/.../config/EngineExtensionsHolder.java: deleted (unreferenced post-#5403; not present on feature). Build: :api, :core, :opensearch-sql-plugin, :legacy compileJava + :integ-test compileTestJava all pass; unit tests pass; spotlessCheck clean. Signed-off-by: Kai Huang <ahkcs@amazon.com>

dai-chen self-assigned this May 6, 2026

dai-chen added PPL Piped processing language bugFix labels May 6, 2026

dai-chen force-pushed the feature/ppl-udt-cleanup branch from 70aabfe to 01fba06 Compare May 6, 2026 18:31

dai-chen marked this pull request as ready for review May 6, 2026 18:32

dai-chen requested review from LantaoJin, RyanL1997, Swiddis, acarbonetto, ahkcs, anirudha, joshuali925, mengweieric, noCharger, penghuo, ps48, qianheng-aws, songkant-aws, vamsimanohar, ykmr1224 and yuancu as code owners May 6, 2026 18:32

penghuo previously approved these changes May 6, 2026

View reviewed changes

mengweieric previously approved these changes May 6, 2026

View reviewed changes

dai-chen dismissed stale reviews from mengweieric and penghuo via cc90d7f May 7, 2026 01:53

dai-chen force-pushed the feature/ppl-udt-cleanup branch from 01fba06 to cc90d7f Compare May 7, 2026 01:53

dai-chen force-pushed the feature/ppl-udt-cleanup branch from cc90d7f to af2b288 Compare May 7, 2026 02:53

dai-chen force-pushed the feature/ppl-udt-cleanup branch from af2b288 to 58b0420 Compare May 7, 2026 15:33

mengweieric approved these changes May 7, 2026

View reviewed changes

penghuo approved these changes May 7, 2026

View reviewed changes

dai-chen merged commit 3ed472e into opensearch-project:main May 7, 2026
37 of 40 checks passed

ahkcs mentioned this pull request May 7, 2026

Datetime output cast format diverges between Calcite and DataFusion engines #5420

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normalize datetime types for unified query API#5408

Normalize datetime types for unified query API#5408
dai-chen merged 1 commit intoopensearch-project:mainfrom
dai-chen:feature/ppl-udt-cleanup

dai-chen commented May 6, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 6, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 6, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 6, 2026

Uh oh!

dai-chen commented May 6, 2026

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

dai-chen commented May 7, 2026

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

dai-chen commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Examples

Notes

Related Issues

Check List

Uh oh!

github-actions Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit af2b288)

Uh oh!

github-actions Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

github-actions Bot commented May 6, 2026

Uh oh!

dai-chen commented May 6, 2026

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

dai-chen commented May 7, 2026

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dai-chen commented May 6, 2026 •

edited

Loading

github-actions Bot commented May 6, 2026 •

edited

Loading

(Review updated until commit `af2b288`)

github-actions Bot commented May 6, 2026 •

edited

Loading