Skip to content

Conversation

ahkcs
Copy link
Contributor

@ahkcs ahkcs commented Oct 7, 2025

Description

Implements unified time unit support across PPL commands with case-sensitive handling.

Key Changes:

  • Fixed case sensitivity for bin command: Uppercase M = month, lowercase m = minute
  • Added subsecond test cases to bin command: us, cs, ds
  • Documented limitations: stats command cannot support subsecond units (OpenSearch calendar intervals)

Related Issues

Resolves #4397

Copy link
Collaborator

@dai-chen dai-chen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we have single place for span parsing logic (currently in AstExpressionBuilder.visitSpan?) instead of trying to parse span string everywhere? Any difference between span in bin options and span elsewhere?

Signed-off-by: Kai Huang <[email protected]>

# Conflicts:
#	integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinCommandIT.java
@ahkcs ahkcs force-pushed the feature/unified-time-units-4397 branch from dbcf2ba to 77d1596 Compare October 15, 2025 19:50
@ahkcs
Copy link
Contributor Author

ahkcs commented Oct 15, 2025

Shall we have single place for span parsing logic (currently in AstExpressionBuilder.visitSpan?) instead of trying to parse span string everywhere? Any difference between span in bin options and span elsewhere?

Thanks @dai-chen for the suggestion, however bin span is indeed different with the span elsewhere
For Span Types:

  • stats/timechart: Numeric + Time only
  • bin: Numeric + Time + Logarithmic (log2, log10, 2.5log3)

I think it's still necessary to separate the implementation of bin span and other span

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent time unit support across PPL commands

2 participants