[bhubam]

Interval buckets such as MULTIPLY(FLOOR(DIVIDE(col, 60000)), 60000) failed on Postgres because function operands could only be columns. Nested functions and numeric constants are now rendered inline, and a function group-by repeats the selected expression.

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.25%. Comparing base (cd897e9) to head (96edd60).

Files with missing lines	Patch %	Lines
.../v1/vistors/PostgresFunctionExpressionVisitor.java	77.77%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #329      +/-   ##
============================================
+ Coverage     81.23%   81.25%   +0.01%     
- Complexity     1698     1700       +2     
============================================
  Files           243      243              
  Lines          7780     7792      +12     
  Branches        771      773       +2     
============================================
+ Hits           6320     6331      +11     
+ Misses          962      961       -1     
- Partials        498      500       +2     

Flag	Coverage Δ
integration	81.25% <80.00%> (+0.01%)	⬆️
unit	58.90% <80.00%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

[github-actions]

Test Results

  128 files  ±0    128 suites  ±0   41s ⏱️ -2s
  900 tests +6    899 ✅ +6  1 💤 ±0  0 ❌ ±0 
1 270 runs  +6  1 269 ✅ +6  1 💤 ±0  0 ❌ ±0 

Results for commit 96edd60. ± Comparison against base commit cd897e9.

♻️ This comment has been updated with latest results.

[suddendust]

Can we add a Mongo/PG compatibility test for this?

[suddendust]

Any way of avoiding these instanceof checks?

[bhubam]

Done in 96edd60. Operands are now rendered by a PostgresFunctionOperandVisitor (inner class of PostgresFunctionExpressionVisitor), so getParsedExpression is just expression.accept(operandVisitor):

• visit(FunctionExpression) renders the nested function wrapped in parentheses
• visit(ConstantExpression) inlines numeric constants
• visit(IdentifierExpression) reuses the aliased selection if present, else falls back to the data-accessor visitor
• anything else falls through to the default visitor, which throws as before

The only instanceof left is value instanceof Number on the constant's value, which is a value check rather than expression-type dispatch. Unit tests and the full DocStoreQueryV1Test suite (Mongo, nested PG, flat PG) pass.

[suddendust]

Can you check if we can use visitors to avoid instanceof checks? That's the pattern we've tried to maintain in this repo.

[suddendust]

Nit (Suggestion from Gemini):

Actionable Feedback: While functionally correct, repeating long, complex nested functions in both the SELECT and GROUP BY clauses can occasionally impact query parsing/planning overhead. Since Postgres supports grouping by ordinal positions (e.g., GROUP BY 1), it might be worth investigating if ordinal grouping could be safely generated here to reduce query payload size and parsing time for highly nested expressions.

[suddendust]

LGTM