[OPIK-3073] [BE] Make to time optional

[thiagohora]

Revision 3: Address GitHub PR Comments - Refactor Statement Creation and Optimize Template Variables

Details

Refactored ProjectMetricsDAO to address code review feedback from GitHub Copilot and @andrescrz, improving efficiency and code quality while maintaining all existing functionality.

Changes Made

1. Fixed Statement Creation Inefficiency (Comments #1 & #2)

Methods affected: getTotalCost() and getAverageDuration()

Problem:

• Template flags were added AFTER the initial render() call
• Statement was created twice when endTime != null, discarding the first instance
• Inefficient pattern that wasted resources

Solution:
// Before (inefficient):

var statement = connection.createStatement(stTemplate.render())
        .bind("uuid_from_time", uuidFromTime);
if (endTime != null) {
    stTemplate.add("uuid_to_time", true);  // Added AFTER render
    var uuidToTime = instantToUUIDMapper.toUpperBound(endTime).toString();
    statement = connection.createStatement(stTemplate.render())  // Created TWICE
            .bind("uuid_from_time", uuidFromTime)
            .bind("uuid_to_time", uuidToTime);
}

// After (optimized):

String uuidToTime = null;
if (endTime != null) {
    stTemplate.add("uuid_to_time", true);  // Added BEFORE render
    uuidToTime = instantToUUIDMapper.toUpperBound(endTime).toString();
}
var statement = connection.createStatement(stTemplate.render())  // Created ONCE
        .bind("uuid_from_time", uuidFromTime);
if (uuidToTime != null) {
    statement = statement.bind("uuid_to_time", uuidToTime);
}**Benefits:**
- Statement created only once with all template flags properly set
- More efficient resource usage
- Cleaner, more maintainable code

2. Removed Unnecessary fill_to Template Variable (Comment #3)

Methods affected: getMetric()

Problem:

• fill_to template variable was always set to the same value
• Unnecessary abstraction that didn't add flexibility

Solution:
// Before:

template.add("fill_to", "TO toDateTime(UUIDv7ToDateTime(toUUID(:uuid_to_time)))");

// SQL template:

<if(with_fill)>WITH FILL
    FROM <fill_from>
    <fill_to>
    STEP <step><endif>;

// After:
// Removed template.add("fill_to", ...) line

// SQL template (hardcoded TO clause):

<if(with_fill)>WITH FILL
    FROM <fill_from>
    TO toDateTime(UUIDv7ToDateTime(toUUID(:uuid_to_time)))
    STEP <step><endif>;

Benefits:

• Simplified code with one less template variable
• TO clause is clearly visible in SQL template
• No loss of functionality

---

3. Made GET_COST and GET_GUARDRAILS_FAILED_COUNT Conditional

Queries affected: GET_COST and GET_GUARDRAILS_FAILED_COUNT

Problem:

• These two queries had unconditional WITH FILL clauses
• Would fail when uuid_to_time is null (optional to_time parameter)
• Inconsistent with other query patterns

Solution:
// Before:

WITH FILL
    FROM <fill_from>
    <fill_to>
    STEP <step>;

// After:

<if(with_fill)>WITH FILL
    FROM <fill_from>
    TO toDateTime(UUIDv7ToDateTime(toUUID(:uuid_to_time)))
    STEP <step><endif>;

Benefits:

• Consistent behavior across all metric queries
• Properly handles optional interval_end parameter
• WITH FILL only applied when both boundaries are provided

Behavioral Impact

When to_time/interval_end is provided:

• with_fill flag set to true
• WITH FILL clause included in SQL
• :uuid_to_time parameter bound and used
• Returns time-series data with null-filled gaps

When to_time/interval_end is omitted (common case):

• with_fill flag remains false
• WITH FILL clause excluded from SQL (via <if(with_fill)>)
• :uuid_to_time parameter not bound (not needed)
• Returns only actual data points without null padding

---

Change checklist

• User facing
• Documentation update

Issues

• Resolves #
  OPIK-3073

Testing

• ✅ All 215 tests passing (0 failures, 1 skipped)
• ✅ Tested with both interval_end provided and omitted
• ✅ Empty data scenarios work correctly
• ✅ All metric types validated

---

Documentation

NA

[github-actions]

SDK E2E Tests Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 60c4612.

[github-actions]

SDK E2E Tests Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 60c4612.

♻️ This comment has been updated with latest results.

[Copilot]

Pull Request Overview

This PR optimizes the ProjectMetricsDAO implementation by refactoring statement creation patterns and SQL template handling. The changes address code review feedback to improve efficiency and consistency across metric queries.

Key changes:

• Optimized statement creation in getTotalCost() and getAverageDuration() to set template flags before rendering, eliminating duplicate statement creation
• Made WITH FILL clauses conditional in GET_COST and GET_GUARDRAILS_FAILED_COUNT queries for consistency with other metric queries
• Removed redundant fill_to template variable by hardcoding the TO clause directly in SQL templates

[github-actions]

Backend Tests Results

  290 files    290 suites   50m 7s ⏱️
5 467 tests 5 460 ✅ 7 💤 0 ❌
5 466 runs  5 459 ✅ 7 💤 0 ❌

Results for commit 60c4612.