Skip to content

implement GREATEST and LEAST functions - #2921

Merged
jennifersp merged 5 commits into
mainfrom
jennifer/est
Jul 14, 2026
Merged

implement GREATEST and LEAST functions#2921
jennifersp merged 5 commits into
mainfrom
jennifer/est

Conversation

@jennifersp

Copy link
Copy Markdown
Contributor

No description provided.

@jennifersp
jennifersp requested a review from Hydrocharged July 10, 2026 23:24
@jennifersp jennifersp linked an issue Jul 10, 2026 that may be closed by this pull request
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor
Main PR
covering_index_scan_postgres 1795.23/s 1789.88/s -0.3%
groupby_scan_postgres 127.22/s 119.26/s -6.3%
index_join_postgres 631.49/s 620.71/s -1.8%
index_join_scan_postgres 783.33/s 783.04/s -0.1%
index_scan_postgres 25.68/s 25.61/s -0.3%
oltp_delete_insert_postgres 824.29/s 762.61/s -7.5%
oltp_insert 675.93/s 672.11/s -0.6%
oltp_point_select 3236.15/s ${\color{red}2826.14/s}$ ${\color{red}-12.7\%}$
oltp_read_only 3128.62/s 2838.06/s -9.3%
oltp_read_write 2487.46/s 2239.70/s -10.0%
oltp_update_index 754.50/s 710.00/s -5.9%
oltp_update_non_index 777.78/s 742.42/s -4.6%
oltp_write_only 1814.30/s 1686.91/s -7.1%
select_random_points 1865.54/s 1811.83/s -2.9%
select_random_ranges 1467.22/s 1455.96/s -0.8%
table_scan_postgres 24.68/s 24.22/s -1.9%
types_delete_insert_postgres 792.90/s 746.98/s -5.8%
types_table_scan_postgres 11.00/s 10.67/s -3.1%

@itoqa

itoqa Bot commented Jul 11, 2026

Copy link
Copy Markdown

Ito QA test results
Commit: f252be9: 5 test cases ran, 2 failed ❌, 1 passed ✅, 2 additional findings ⚠️.

Summary

This run exercised SQL query behavior for computed expressions across normal query flows and edge cases, including numeric comparison functions, ordering behavior, and default column-name formatting. Overall health is mixed: baseline expression handling looked stable in some paths, but key comparison-function behavior remains broken in core query execution.

Not safe to merge yet — this PR is tied to multiple unresolved functional failures, including a high-severity break in core numeric comparison behavior and an additional medium-severity failure in the same query path, which makes common query patterns unreliable. There are also separate non-attributable column-label formatting issues that are worth tracking but are not the merge driver for this change.

Tests run by Ito

View full run

Result Severity Type Description
High severity Function The function call fails with an unsupported type error for Doltgres numeric types, and the PR source also contains unresolved references for the new dedicated dispatch path.
Medium severity Select The test expected unaliased computed expressions including greatest/least to return rows with normal labels, but greatest/least consistently throws an internal type error: unsupported type for greatest/least argument: *types.DoltgresType.
Function ORDER BY handling stayed consistent: generic functions and supported GREATEST/LEAST forms succeeded, while numeric GREATEST/LEAST failed with the same function-level type error seen outside ORDER BY.
⚠️ Medium severity Projection Wire field-description names remain quoted (for example "current_date") for reserved-keyword functions, while baseline expressions such as now() and 1+1 are unquoted and aliases behave correctly.
⚠️ Minor severity Projection The observed headers are "current_date", "current_user", and "current_timestamp" (with outer quotes) instead of trimmed names, while control expressions return unquoted labels.
Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

🟡 Unaliased reserved function column names leak quotes
  • Severity: Medium Medium severity
  • Description: Wire field-description names remain quoted (for example "current_date") for reserved-keyword functions, while baseline expressions such as now() and 1+1 are unquoted and aliases behave correctly.
  • Impact: Some query consumers may mis-handle result columns for unaliased reserved-keyword functions because returned field names include unexpected outer quotes. This can break downstream mapping or projection logic until explicit aliases are added.
  • Steps to Reproduce:
    1. Run unaliased expressions such as SELECT current_date, SELECT current_user, and SELECT current_timestamp and inspect column metadata from psql/psycopg2.
    2. Repeat the same expression through CTE, subquery, VALUES, and UNION query shapes.
    3. Compare observed wire field names against normalized expression labels expected from InputExpression trimming.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: In server/ast/select.go, inputExpressionForSelectExpr trims outer quotes from the computed InputExpression (lines 173-181), but wire field emission is later produced in server/doltgres_handler.go by copying schema column name c.Name directly into FieldDescription.Name (lines 511-516 and 542-543) with no quote-normalization pass. This explains why quote trimming can exist at AST conversion time while quoted names still reach client-visible wire metadata.
Evidence Package
⚪ Unaliased computed expression still returns quoted reserved-keyword headers
  • Severity: Minor Minor severity
  • Description: The observed headers are "current_date", "current_user", and "current_timestamp" (with outer quotes) instead of trimmed names, while control expressions return unquoted labels.
  • Impact: Users may see quoted column headers for unaliased reserved-keyword expressions, which can cause confusing result labels in query output.
  • Steps to Reproduce:
    1. Connect to the local Doltgres instance as postgres.
    2. Run unaliased computed projections: SELECT current_date; SELECT current_user; SELECT current_timestamp;.
    3. Inspect the returned column headers or cursor description names.
    4. Compare with controls SELECT now(); and SELECT 1+1; to confirm normalization behavior differs for reserved-keyword functions.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The PR-added trim in server/ast/select.go strips outer double quotes from InputExpression, but quoting is reintroduced later by identifier formatting rules (Name.Format -> lex.EncodeRestrictedSQLIdent), which quote reserved keywords. The existing skipped current_schema column-name test indicates this quoted-header behavior already existed; the changed lines in this PR attempt normalization but do not fully control the final emitted field name path.
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

Comment thread server/ast/func_expr.go
Comment thread server/ast/func_expr.go

@Hydrocharged Hydrocharged left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think you may have forgotten to include the expressions

Comment thread server/ast/select.go
Comment thread server/ast/func_expr.go
@itoqa

itoqa Bot commented Jul 13, 2026

Copy link
Copy Markdown

Ito QA test results
Ito Diff Reportf252be9fa20690: 15 test cases ran, 3 fixed ✅, 11 passing ✅, 1 additional finding ⚠️.

Diff Summary

This run covered core SQL behavior around GREATEST/LEAST in regular reads, filtering, and update-returning flows, plus mixed-type handling and invalid-input paths. The application behavior was broadly stable across happy-path computation, boundary/error handling, and metadata naming checks.

Safe to merge — there are no regressions, new failures, or previously flagged still-failing issues attributable to this PR, and the exercised behaviors remained stable. The only failure is a minor unrelated additional finding, which is a follow-up item rather than a merge blocker.

Tests run by Ito

View full run

Result State Severity Type Description
❌->✅ Fixed Function SELECT GREATEST(a, b, c, NULL) returned the expected highest non-NULL values (12.34500 and 456.78901) with default column label greatest. This matches the intended dispatch and evaluation path for greatest/least function handling.
❌->✅ Fixed Projection Direct SQL verification shows unaliased GREATEST/LEAST projection labels are normalized to unquoted names (for example, greatest/least), matching the expected metadata behavior.
❌->✅ Fixed Select Computed greatest/least expressions returned expected rows, and unaliased projections used normalized column names greatest and least.
Passing Function ORDER BY inside function calls is consistently rejected: abs() fails with a semantic unsupported-function-order-by error, while GREATEST/LEAST fail earlier with a parser syntax error. Code inspection confirms the greatest/least dispatch branch does not accept ORDER BY execution, and observed behavior remained stable with no planner bypass.
Passing Function UPDATE d SET v = v + 1 RETURNING LEAST(v, 5) AS bounded returned 5 and 4 as expected, and post-update verification showed stored values changed to 11 and 4 with no conversion error.
Passing Function SELECT a, b FROM w WHERE LEAST(a, b) < 5 ORDER BY a returned rows (4,7) and (8,2), matching expected predicate filtering and confirming LEAST evaluation in predicate context.
Passing Function SELECT LeAsT(14, 4, 9) AS v; returned 4, confirming mixed-case function naming still dispatches through the LEAST injected evaluator path.
Passing Function UPDATE ... RETURNING LEAST(x, 'bad') produced an int-cast syntax error while the NOTAFUNCTION control produced a distinct function-not-found error, confirming LEAST dispatch reached the evaluator path as expected.
Passing Type LEAST(a, b, c) returned the correct minimum value 2.75 for decimal(6,2), decimal(8,5), and decimal(5,1) inputs, and the result remained numeric-compatible via cast probe despite pg_typeof() being unavailable in this build.
Passing Type LEAST(5, 'abc') is rejected with clear type-resolution errors and returns no data row, matching expected behavior for incompatible argument categories.
Passing Type The plan-specific PREPARE/EXECUTE plus pg_typeof path is unavailable in this build, but the same unknown-parameter execution path was verified through bound parameters and returned the expected lexical maximum ('beta') with successful text-column insertion.
Passing Type Greatest/Least malformed and edge-case inputs returned typed errors or valid results without crashing the service, and the server remained available after the run.
Passing Type Mixed-type GREATEST/LEAST queries were executed through direct SQL, subquery, CTE, view, UNION, CASE, COALESCE, transaction, and repeated runs, with consistent outputs for identical inputs. The negative text+int case returned the same typed errno 1105 error across paths, and the server remained responsive after execution.
Passing Type Mixed-precision NUMERIC inputs returned symmetric GREATEST/LEAST results across argument orderings, coherent extrema, correct NULL handling, and consistent values under explicit casts.
⚠️ Additional Finding Minor severity Projection The same normalized expression label is emitted differently at the wire boundary when wrapped as a scalar subquery, reintroducing double quotes that were normalized away in direct projection output.
Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

⚪ Wrapped subquery field name re-quotes normalized function labels
  • Severity: Minor Minor severity
  • Description: The same normalized expression label is emitted differently at the wire boundary when wrapped as a scalar subquery, reintroducing double quotes that were normalized away in direct projection output.
  • Impact: The projection flow can stall and never finish when this condition occurs, which can prevent users from completing the operation until they retry.
  • Steps to Reproduce:
    1. Connect to Doltgres with psql against the local test database.
    2. Run SELECT GREATEST(1, 2, 3); and observe the unaliased column header is greatest.
    3. Run SELECT (SELECT GREATEST(1, 2, 3)); and observe the returned column header is (SELECT "greatest") instead of (SELECT greatest).
    4. Repeat with LEAST to confirm the same drift pattern as (SELECT "least").
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: server/ast/select.go computes InputExpression via tree.AsStringWithFlags and only trims quotes when the entire expression string starts and ends with matching quotes. For scalar-subquery expressions, the serialized text begins with parentheses, so embedded quoted identifiers like "greatest" remain untouched. Later, server/doltgres_handler.go schemaToFieldDescriptions forwards schema column names directly to FieldDescription.Name (except the string-literal sentinel case), so the re-quoted subquery form reaches clients unchanged. A targeted fix is to normalize quoted identifiers inside subquery-derived projection names (or emit a canonical alias) before FieldDescription serialization, while preserving intentional quoted identifiers.
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor
Main PR
Total 42090 42090
Successful 18275 18277
Failures 23815 23813
Partial Successes1 5335 5265
Main PR
Successful 43.4189% 43.4236%
Failures 56.5811% 56.5764%

${\color{lightgreen}Progressions (2)}$

join

QUERY: select * from
  int8_tbl a left join lateral
  (select b.q1 as bq1, c.q1 as cq1, least(a.q1,b.q1,c.q1) from
   int8_tbl b cross join int8_tbl c) ss
  on a.q2 = ss.bq1;
QUERY: select t1.b, ss.phv from join_ut1 t1 left join lateral
              (select t2.a as t2a, t3.a t3a, least(t1.a, t2.a, t3.a) phv
					  from join_pt1 t2 join join_ut1 t3 on t2.a = t3.b) ss
              on t1.a = ss.t2a order by t1.a;

Footnotes

  1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct.

@jennifersp
jennifersp requested a review from Hydrocharged July 13, 2026 19:16

@Hydrocharged Hydrocharged left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

Comment thread server/expression/greatest_least.go Outdated
Comment thread server/expression/greatest_least.go Outdated
Comment thread server/expression/greatest_least.go Outdated
@itoqa

itoqa Bot commented Jul 14, 2026

Copy link
Copy Markdown

Ito QA test results
Ito Diff Reportfa2069020a2864: 13 test cases ran, 1 fixed ✅, 12 passing ✅.

Diff Summary

This run exercised core SQL behavior for greatest/least-style comparisons across normal query flows and edge cases, including mixed data types, NULL handling, large argument lists, expression naming in results, and use inside update-returning and filtering contexts. It also covered defensive behavior, confirming invalid type combinations fail predictably while valid combinations stay consistent and stable across repeated and reordered inputs.

Safe to merge — this run shows no regressions, no new failures, and no still-failing issues attributed to this PR. The changed behavior appears stable across both expected usage and error-path conditions, so current merge risk is low.

Tests run by Ito

View full run

Result State Severity Type Description
❌->✅ Fixed Projection All 13 query variants returned column names consistent with InputExpression normalization across planner and handler paths, including cast, function, literal, CTE, and subquery cases.
Passing Function ORDER BY clauses inside function calls were rejected for both generic scalar functions and greatest/least, with no acceptance path that would bypass planner safeguards.
Passing Function UPDATE d SET v = v + 1 RETURNING LEAST(v, 5) returned bounded values 5 and 4 without planner or conversion errors.
Passing Type LEAST and GREATEST returned the expected decimal extrema for mixed precision columns without cast failures.
Passing Type Incompatible GREATEST/LEAST type combinations correctly failed with cast-matching errors while compatible combinations succeeded.
Passing Type GREATEST and LEAST both succeed and return correct opposite extrema (g=8.80000, l=2.75) regardless of argument ordering. Behavior parity confirmed after file split.
Passing Type GREATEST/LEAST with mixed text+int types fails deterministically with SQLSTATE HY000 error 1105 at setup boundary. Control with all-cast-compatible values returns complete results. Per-row lookup is NOT happening; the failure occurs at the type-resolution stage before any rows are processed.
Passing Type Unknown literals in GREATEST resolve to text comparison deterministically, so lexical winners like '20' over '100' are expected and consistently typed rather than a silent runtime defect.
Passing Type Repeated execution of the same mixed-type query consistently fails at the same stage with stable SQLSTATE/errno, and the plan-time vs runtime difference only appears when query shape changes between typed columns and unknown literals.
Passing Type Baseline-versus-current comparison found only import-order changes and no behavioral drift for GREATEST/LEAST mixed-type workloads.
Passing Type Malformed non-expression children are rejected with typed errors, malformed context inputs only panic inside the reflection harness call site, and the server remains available for subsequent SQL queries.
Passing Type GREATEST(NULL::int, NULL::int, NULL::int) and LEAST(NULL::int, NULL::int) both returned SQL NULL with no cast or comparison errors.
Passing Type 50-argument GREATEST/LEAST calls succeeded with stable extrema (max=100, min=1) across original and reordered inputs, and the session remained responsive.
⏸️ Skipped Function SELECT GREATEST(a, b, c, NULL) returned the expected highest non-NULL values (12.34500 and 456.78901) with default column label greatest. This matches the intended dispatch and evaluation path for greatest/least function handling.
⏸️ Skipped Function SELECT a, b FROM w WHERE LEAST(a, b) < 5 ORDER BY a returned rows (4,7) and (8,2), matching expected predicate filtering and confirming LEAST evaluation in predicate context.
⏸️ Skipped Function SELECT LeAsT(14, 4, 9) AS v; returned 4, confirming mixed-case function naming still dispatches through the LEAST injected evaluator path.
⏸️ Skipped Function UPDATE ... RETURNING LEAST(x, 'bad') produced an int-cast syntax error while the NOTAFUNCTION control produced a distinct function-not-found error, confirming LEAST dispatch reached the evaluator path as expected.
⏸️ Skipped Projection Direct SQL verification shows unaliased GREATEST/LEAST projection labels are normalized to unquoted names (for example, greatest/least), matching the expected metadata behavior.
⏸️ Skipped Select Computed greatest/least expressions returned expected rows, and unaliased projections used normalized column names greatest and least.
⏸️ Skipped Type LEAST(5, 'abc') is rejected with clear type-resolution errors and returns no data row, matching expected behavior for incompatible argument categories.
⏸️ Skipped Type The plan-specific PREPARE/EXECUTE plus pg_typeof path is unavailable in this build, but the same unknown-parameter execution path was verified through bound parameters and returned the expected lexical maximum ('beta') with successful text-column insertion.
⏸️ Skipped Type Mixed-type GREATEST/LEAST queries were executed through direct SQL, subquery, CTE, view, UNION, CASE, COALESCE, transaction, and repeated runs, with consistent outputs for identical inputs. The negative text+int case returned the same typed errno 1105 error across paths, and the server remained responsive after execution.
⏸️ Skipped Type Mixed-precision NUMERIC inputs returned symmetric GREATEST/LEAST results across argument orderings, coherent extrema, correct NULL handling, and consistent values under explicit casts.

Tip

Reply with @itoqa to send us feedback on this test run.

@jennifersp
jennifersp merged commit 7886f0f into main Jul 14, 2026
25 of 26 checks passed
@jennifersp
jennifersp deleted the jennifer/est branch July 14, 2026 20:55
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.

Handle Doltgres types in least/greatest functions

2 participants