Skip to content

bug fix: load non-built-in types when looking for an array type's element type#2729

Merged
fulghum merged 4 commits into
mainfrom
fulghum/bugfix
May 20, 2026
Merged

bug fix: load non-built-in types when looking for an array type's element type#2729
fulghum merged 4 commits into
mainfrom
fulghum/bugfix

Conversation

@fulghum

@fulghum fulghum commented May 18, 2026

Copy link
Copy Markdown
Contributor

Various bug fixes for regressions reported in PR #2642, primarily related to resolving arrays of non-built-in types. The major change is adding ctx to the ArrayBaseType() method, but not all call sites could be updated, so unfortunately results in a new ArrayBaseTypeCtx(ctx) method until we can find a way to always provide a sql context and completely remove the old method form.

@github-actions

github-actions Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor
Main PR
covering_index_scan_postgres 1346.35/s 1320.55/s -2.0%
index_join_postgres 190.44/s 189.50/s -0.5%
index_join_scan_postgres 198.58/s 197.81/s -0.4%
index_scan_postgres 12.49/s 12.35/s -1.2%
oltp_point_select 2428.82/s 2431.24/s 0.0%
oltp_read_only 1826.32/s 1808.58/s -1.0%
select_random_points 130.68/s 128.62/s -1.6%
select_random_ranges 888.84/s 869.57/s -2.2%
table_scan_postgres 12.19/s 12.06/s -1.1%
types_table_scan_postgres 5.61/s 5.52/s -1.7%

@github-actions

github-actions Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor
Main PR
Total 42090 42090
Successful 18104 18125
Failures 23986 23965
Partial Successes1 5377 5384
Main PR
Successful 43.0126% 43.0625%
Failures 56.9874% 56.9375%

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

arrays

QUERY: insert into comptable
  values (row(1,'foo'), array[row(2,'bar')::comptype, row(3,'baz')::comptype]);
QUERY: select * from comptable;
QUERY: insert into dest select array[row(f1,f1)::textandtext] from src;

create_function_sql

QUERY: CREATE FUNCTION double_append(anyarray, anyelement) RETURNS SETOF anyarray
LANGUAGE SQL IMMUTABLE AS
$$ SELECT array_append($1, $2) || array_append($1, $2) $$;

domain

QUERY: insert into dcomptable values (null);
QUERY: select array[1,2]::orderedpair;

enum

QUERY: SELECT '{red,green,blue}'::rainbow[];
QUERY: SELECT ('{red,green,blue}'::rainbow[])[2];

plpgsql

QUERY: select f1(stavalues1) from pg_statistic;
QUERY: select f1(stavalues1) from pg_statistic;

polymorphism

QUERY: select polyf(stavalues1) from pg_statistic;
QUERY: select polyf(stavalues1) from pg_statistic;
QUERY: CREATE FUNCTION tfp(anyarray,anyelement) RETURNS anyarray AS
'select $1 || $2' LANGUAGE SQL;
QUERY: create function first_el_transfn(anyarray, anyelement) returns anyarray as
'select $1 || $2' language sql immutable;
QUERY: create function formarray(anyelement, variadic anyarray) returns anyarray as $$
  select array_prepend($1, $2);
$$ language sql immutable strict;
QUERY: drop function formarray(anyelement, variadic anyarray);

rowtypes

QUERY: select row('Joe', 'Blow')::fullname, '(Joe,Blow)'::fullname;
QUERY: select '(Joe,von Blow)'::fullname, '(Joe,d''Blow)'::fullname;
QUERY: select '(Joe,"Blow,Jr")'::fullname;
QUERY: select '(Joe,)'::fullname;
QUERY: insert into people values ('(Joe,Blow)', '1984-01-10');

updatable_views

QUERY: create view wcowrtest_v as select * from wcowrtest where wcowrtest = '(2)'::wcowrtest with check option;

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.

@fulghum fulghum marked this pull request as ready for review May 19, 2026 23:22
@fulghum fulghum requested a review from Hydrocharged May 19, 2026 23:43
@itoqa

itoqa Bot commented May 20, 2026

Copy link
Copy Markdown

Ito Test Report ❌

13 test cases ran. 1 failed, 2 additional findings, 10 passed.

Overall, 10 of 13 test cases passed, showing broad stability in schema/type resolution, context-aware array base-type handling, composite-array parsing, malformed-literal rejection, and server resilience after controlled errors. The most important findings are three high-severity backend defects: a newly introduced panic risk when serializing SQL function results of user-defined composite arrays (aggtype[]) and two pre-existing polymorphic anyarray invocation/overload failures that raise unbound variable "v1" at runtime.

❌ Failed (1)
Category Summary Screenshot
Resolve ⚠️ Calling a SQL function that returns aggtype[] can panic during array serialization instead of returning rows. RESOLVE-2
⚠️ Composite array function call panics during result serialization
  • What failed: Instead of returning the expected aggtype[] rows, function invocation can panic while deriving the array base element type during serialization.
  • Impact: SQL functions that operate on user-defined composite arrays can fail at runtime and break core query workflows. Teams relying on composite-array function pipelines may hit non-recoverable execution failures.
  • Steps to reproduce:
    1. Create aggtype and define a SQL function that takes and returns aggtype[].
    2. Invoke the function with ARRAY[ROW(1,2,'hello')::aggtype] as input.
    3. Observe runtime behavior while serializing the returned array value.
  • Stub / mock context: SCRAM authentication was disabled so the SQL function scenario could run without login gating; no route-level response mocking was used for this defect check.
  • Code analysis: I traced the failing scenario from the create-function SQL regression test into array serialization and base-type resolution. The serialization path now calls context-aware base-type derivation, but that resolver still panics when built-in, logical, and context collection lookups do not resolve the element type.
  • Why this is likely a bug: Production code contains an explicit panic in the array base-type resolution path reached by normal aggtype[] function serialization, so this is a real application defect rather than test setup noise.

Relevant code:

testing/go/create_function_sql_test.go (lines 420-429)

{
	Query:    `CREATE FUNCTION aggf_trans(aggtype[],integer,integer,text) RETURNS aggtype[] AS 'select array_append($1,ROW($2,$3,$4)::aggtype)' LANGUAGE sql STRICT IMMUTABLE`,
	Expected: []sql.Row{},
},
{
	Query:    `SELECT aggf_trans(ARRAY[]::aggtype[], 1, 2, 'hello')`,
	Expected: []sql.Row{{`{"(1,2,hello)"}`}},
},
{
	Query:    `SELECT aggf_trans(ARRAY[ROW(1,2,'hello')::aggtype], 3, 4, 'world')`,
	Expected: []sql.Row{{`{"(1,2,hello)","(3,4,world)"}`}},
},

server/types/array.go (lines 80-84)

// serializeTypeArray handles serialization from the standard representation to our serialized representation that is
// written in Dolt.
func serializeTypeArray(ctx *sql.Context, t *DoltgresType, val any) ([]byte, error) {
	return serializeArray(ctx, val.([]any), t.ArrayBaseTypeCtx(ctx))
}

server/types/type.go (lines 169-180)

if t.IsUnresolved && t.Elem != id.NullType {
	return NewUnresolvedDoltgresType(t.Elem.SchemaName(), t.Elem.TypeName())
}
if ctx != nil && t.Elem != id.NullType && GetTypesCollectionFromContext != nil {
	if typeColl, err := GetTypesCollectionFromContext(ctx); err == nil && typeColl != nil {
		if elemType, err := typeColl.GetType(ctx, t.Elem); err == nil && elemType != nil {
			newElem := *elemType.WithAttTypMod(t.attTypMod)
			return &newElem
		}
	}
}
panic(fmt.Sprintf("cannot get base type from: %s", t.Name()))
✅ Passed (10)
Category Summary Screenshot
Composite Not a real application bug in current codebase; re-execution of the SQL flow succeeded end-to-end with correctly typed aggtype[] output. COMPOSITE-1
Composite Malformed literals failed immediately with explicit arity mismatch errors for too-few and too-many fields. COMPOSITE-2
Composite Malformed composite/array literals returned controlled parser errors and the backend remained responsive. COMPOSITE-3
Composite Mixed composite[] insert failed atomically on invalid input, then succeeded cleanly after correction. COMPOSITE-4
Context Domain and enum array literals inserted and selected successfully; output round-tripped as expected (ab,cd and sad,ok), confirming runtime base-type resolution for custom array element types. N/A
Context All malformed array literals failed with explicit parse/lexical errors and did not insert any data (COUNT(*)=0), demonstrating failure is critical and state remains consistent. N/A
Context When custom type resolution was forced into an unresolved path, the engine returned an explicit type error instead of crashing; immediate subsequent query execution succeeded, indicating no panic escaped and process health was preserved. N/A
Resolve Unqualified casts and ARRAY constructors resolved to the active schema's same-named type in each search_path context. RESOLVE-1
Resolve Blank-schema array type fallback correctly resolved to the current schema (s2) for acctype[]. RESOLVE-3
Resolve Inferred unresolved ARRAY casts matched explicit schema-qualified casts across schema-ordering contexts. RESOLVE-4
ℹ️ Additional Findings (2)

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

Category Summary Screenshot
Polymorphic ⚠️ SQL anyarray invocation fails with unbound variable v1 when called with unknown/composite array literal. POLYMORPHIC-1
Polymorphic ⚠️ anyarray overload/coercion path fails with unbound variable v1, blocking remap-before-cast validation. POLYMORPHIC-2
⚠️ Polymorphic anyarray invocation fails on unknown literal
  • What failed: The function call errors with unbound variable "v1" after successful function creation, but this scenario is expected to execute and return a typed array result.
  • Impact: Polymorphic SQL functions that rely on anyarray argument coercion can fail at runtime even when creation succeeds. This breaks a core type-resolution workflow with no reliable workaround for affected function signatures.
  • Steps to reproduce:
    1. Create aggtype and a SQL-language function that accepts anyarray.
    2. Invoke it with an unknown/text literal representation of an aggtype[] input.
    3. Observe invocation failure instead of typed array output.
  • Stub / mock context: Authentication was intentionally disabled so local SQL checks could run deterministically without SCRAM login dependencies, and verification was rerun on a locally rebuilt database image using a newer Go toolchain for compatibility. No API stubs, route interception, or synthetic response mocking was used for this SQL path.
  • Code analysis: I traced SQL-function execution through argument substitution and found that function parameters are stored with declared types (f.ParameterTypes), so anyarray stays as a pseudo-type during formatting. The formatter only auto-quotes array literals when the type category is ArrayTypes, which means anyarray-typed values can be rendered without array-literal quoting and produce invalid downstream expression binding during execution.
  • Why this is likely a bug: Production code still carries anyarray as the parameter type during SQL text substitution, so the execution path can build malformed argument SQL for polymorphic array inputs and reproducibly fails.

Relevant code:

server/functions/framework/sql_function.go (lines 103-116)

paramMap := make(map[string]*ParamTypAndValue)
for i, name := range f.ParameterNames {
	formattedVar, err := f.ParameterTypes[i].FormatValueWithContext(ctx, args[i])
	if err != nil {
		return nil, err
	}
	if name == "" {
		// sanity check
		name = fmt.Sprintf("$%d", i+1)
	}
	paramMap[name] = &ParamTypAndValue{
		Typ:    f.ParameterTypes[i],
		StrVal: formattedVar,
	}
}

postgres/parser/sem/tree/create_function.go (lines 319-327)

if f.StrVal == "" {
	ctx.WriteString(f.Name)
}
// only used when generating query with parameters replaced with actual value
switch f.Typ.TypCategory {
case pgtypes.TypeCategory_ArrayTypes, pgtypes.TypeCategory_DateTimeTypes, pgtypes.TypeCategory_StringTypes, pgtypes.TypeCategory_UserDefinedTypes:
	ctx.WriteString(pq.QuoteLiteral(f.StrVal))
default:
	ctx.WriteString(f.StrVal)
}

server/functions/framework/compiled_function.go (lines 309-315)

// When the declared parameter type is anyarray, the implicit cast from an
// unknown/text argument (via UseInOut) would target anyarray.IoInput which
// cannot be loaded as a QuickFunction. Resolve to the concrete array type
// (e.g. _aggtype) so the cast uses the real element-type I/O path instead.
if targetType.ID == pgtypes.AnyArray.ID {
	targetType = c.resolvePolymorphicReturnType(targetParamTypes, exprTypes, targetType)
}
⚠️ Anyarray overload coercion fails before resolution
  • What failed: Invocation fails with unbound variable "v1" before stable overload/coercion behavior can be evaluated, while expected behavior is deterministic remap-before-cast resolution with successful calls.
  • Impact: Overload resolution scenarios involving anyarray cannot complete reliably, so callers see hard runtime failures instead of predictable coercion behavior. This makes polymorphic function behavior unstable in real query workloads.
  • Steps to reproduce:
    1. Create overloaded functions for anyarray and text[] paths.
    2. Invoke with unknown/text literals and explicit array literals.
    3. Compare overload resolution and execution behavior.
  • Stub / mock context: Authentication was intentionally disabled so local SQL checks could run deterministically without SCRAM login dependencies, and verification was rerun on a locally rebuilt database image using a newer Go toolchain for compatibility. No API stubs, route interception, or synthetic response mocking was used for this SQL path.
  • Code analysis: The same SQL-function substitution path is used for this overload case, and it preserves pseudo-type metadata for parameter rendering. Because runtime substitution is not consistently using the concrete resolved array type for quoted literal generation, overload/coercion calls can fail before intended remap logic is observed.
  • Why this is likely a bug: The failure reproduces on production SQL-function execution logic where polymorphic array arguments are substituted with pseudo-type-driven formatting, which is incompatible with stable overload evaluation.

Relevant code:

server/functions/framework/sql_function.go (lines 103-116)

paramMap := make(map[string]*ParamTypAndValue)
for i, name := range f.ParameterNames {
	formattedVar, err := f.ParameterTypes[i].FormatValueWithContext(ctx, args[i])
	if err != nil {
		return nil, err
	}
	if name == "" {
		// sanity check
		name = fmt.Sprintf("$%d", i+1)
	}
	paramMap[name] = &ParamTypAndValue{
		Typ:    f.ParameterTypes[i],
		StrVal: formattedVar,
	}
}

postgres/parser/sem/tree/create_function.go (lines 319-327)

if f.StrVal == "" {
	ctx.WriteString(f.Name)
}
// only used when generating query with parameters replaced with actual value
switch f.Typ.TypCategory {
case pgtypes.TypeCategory_ArrayTypes, pgtypes.TypeCategory_DateTimeTypes, pgtypes.TypeCategory_StringTypes, pgtypes.TypeCategory_UserDefinedTypes:
	ctx.WriteString(pq.QuoteLiteral(f.StrVal))
default:
	ctx.WriteString(f.StrVal)
}

server/functions/framework/compiled_function.go (lines 309-315)

// When the declared parameter type is anyarray, the implicit cast from an
// unknown/text argument (via UseInOut) would target anyarray.IoInput which
// cannot be loaded as a QuickFunction. Resolve to the concrete array type
// (e.g. _aggtype) so the cast uses the real element-type I/O path instead.
if targetType.ID == pgtypes.AnyArray.ID {
	targetType = c.resolvePolymorphicReturnType(targetParamTypes, exprTypes, targetType)
}

Commit: 9f95337

View Full Run


Tell us how we did: Give Ito Feedback

@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.

Overall looks fine, but definitely have some comments/questions. Additionally, it seems like the primary reason that we needed to thread context through ArrayBaseType was due to not fully resolving types upon their creation. This is something I'm running into concurrently and am working on changing that.

Comment thread server/functions/array.go Outdated
Comment thread server/functions/framework/compiled_function.go
Comment thread server/types/globals.go Outdated
Comment thread server/types/utils.go
@itoqa

itoqa Bot commented May 20, 2026

Copy link
Copy Markdown

Ito Diff Report ❌

Tested: 9f9533733de1d2
14 test cases ran this commit: 6 passed ✅, 2 fixed 🔧, 5 failed ❌, 1 additional finding ⚠️.
↪️ Carried forward from prior run (not retested this commit): 7 passing.

Overall, the unified run failed with 14 total tests, with 8 passing checks confirming stable behavior for many polymorphic, domain/enum-array, NULL-handling, ANY-predicate, and parser-recovery scenarios, but 6 reproducible failures remained. The most important findings were real production defects where _anyelement is incorrectly still accepted (and can later fail at invocation), plus multiple high-impact panic/crash paths in composite/custom array handling (serialization val.([]any) panic, ArrayBaseTypeCtx “cannot get base type” panic, and unguarded type-collection callback wiring), with several issues explicitly identified as introduced by the PR while others appear pre-existing.

❌ Failures (5)
Origin Category Severity Summary Screenshot
🆕 New Context ⚠️ High Multiple unguarded callback lookup boundaries can panic when type-collection wiring is missing or delayed. CONTEXT-6
🆕 New Polymorphic 🟠 Medium _anyelement is still accepted in casts and function signatures instead of failing type lookup. POLYMORPHIC-3
🆕 New Polymorphic 🟠 Medium Legacy _anyelement definitions are accepted but later fail at invocation with function-not-found errors. POLYMORPHIC-5
🔁 Regression Context 🟠 Medium Array base-type resolution can still panic in unresolved custom-type paths instead of returning a controlled SQL error. CONTEXT-3
🔻 Still broken (verified) Resolve ⚠️ High Composite array function call crashes during result serialization instead of returning aggtype[] output. RESOLVE-2
⚠️ Unguarded callback lookups can panic across array and composite boundaries
  • What failed: These boundaries invoke context type-collection lookups without defensive guards, so missing wiring can produce panic behavior instead of controlled SQL error responses.
  • Impact: Core array and composite-type query paths can fail via panic when initialization wiring is not available, affecting multiple workflows with no reliable user-level workaround in affected runtime contexts. This creates broad stability risk across type resolution boundaries.
  • Steps to reproduce:
    1. Start the engine in a mode where type-collection callback wiring is missing or delayed.
    2. Execute one query path through array_in, one through ArrayBaseTypeCtx, and one through composite literal parsing.
    3. Confirm failures can surface as panic behavior instead of controlled SQL errors.
  • Stub / mock context: Real SCRAM authentication was bypassed by disabling the auth gate in server/authentication_scram.go, and this scenario intentionally stress-tested missing or delayed type-collection wiring to verify whether runtime failures stayed graceful.
  • Code analysis: I inspected the changed lookup boundaries and found direct invocation of context lookup in multiple production paths without nil/function-availability checks. Combined with the explicit panic fallback in array base-type derivation, this creates multi-path crash risk when callback wiring is absent or delayed.
  • Why this is likely a bug: Production code now relies on unguarded callback-based lookups across several execution boundaries, making panic outcomes plausible when initialization ordering is imperfect.

Relevant code:

server/functions/array.go (lines 49-53)

baseType := pgtypes.IDToBuiltInDoltgresType[id.Type(baseTypeOid)]
if baseType == nil {
	if typColl, err := pgtypes.GetTypesCollectionFromContext(ctx); err == nil && typColl != nil {
		if t, err := typColl.GetType(ctx, id.Type(baseTypeOid)); err == nil {
			baseType = t
		}
	}
}

server/types/type.go (lines 172-180)

if ctx != nil && t.Elem != id.NullType {
	if typeColl, err := GetTypesCollectionFromContext(ctx); err == nil && typeColl != nil {
		if elemType, err := typeColl.GetType(ctx, t.Elem); err == nil && elemType != nil {
			newElem := *elemType.WithAttTypMod(t.attTypMod)
			return &newElem
		}
	}
}
panic(fmt.Sprintf("cannot get base type from: %s", t.Name()))

server/types/utils.go (lines 265-270)

if t, ok := IDToBuiltInDoltgresType[attr.TypeID]; ok {
	fieldType = t
} else {
	if typColl, tcErr := GetTypesCollectionFromContext(ctx); tcErr == nil && typColl != nil {
		if t, gErr := typColl.GetType(ctx, attr.TypeID); gErr == nil && t != nil {
			fieldType = t
		}
	}
}
🟠 Removed _anyelement alias still resolves
  • What failed: _anyelement should be rejected after alias removal, but both cast parsing and function declaration still accept it.
  • Impact: Migration and validation workflows can silently accept a pseudo-type alias that is supposed to be unsupported, leading to inconsistent type semantics in user SQL. This increases the chance of latent runtime breakage when those definitions are executed later.
  • Steps to reproduce:
    1. Connect to the local database as postgres.
    2. Run SELECT NULL::_anyelement and then CREATE FUNCTION bad_alias_p3(x _anyelement) RETURNS text LANGUAGE SQL AS $$ SELECT 'x' $$.
    3. Compare with SELECT NULL::anyelement IS NULL and verify _anyelement was accepted instead of rejected.
  • Stub / mock context: Authentication was intentionally bypassed by disabling SCRAM checks in server/authentication_scram.go so SQL engine behavior could be tested deterministically without login setup. No route mocking or API stubbing was applied to this test's type-resolution checks.
  • Code analysis: I reviewed built-in type registration and dynamic type lookup paths. The explicit _anyelement built-in mapping was removed, but generic underscore fallback logic still rewrites _name into an array-of-name lookup, which keeps _anyelement valid through anyelement -> anyarray derivation.
  • Why this is likely a bug: The code still provides a production path that resolves _anyelement despite the PR's alias-removal intent, so unsupported syntax is not actually rejected.

Relevant code:

server/types/globals.go (lines 203-208)

toInternal("any"):         Any,
toInternal("anyarray"):    AnyArray,
toInternal("anyelement"):  AnyElement,
toInternal("anyenum"):     AnyEnum,
toInternal("anynonarray"): AnyNonArray,
toInternal("anyrange"):    Unknown,

core/typecollection/typecollection.go (lines 187-195)

// A name starting with "_" may be the implicit array type for a table's composite row
// type. Resolve the element type first (which handles the table lookup), then wrap it.
if strings.HasPrefix(typeName, "_") {
    elemType, err := pgs.GetType(ctx, id.NewType(name.SchemaName(), typeName[1:]))
    if err != nil || elemType == nil {
        return nil, err
    }
    return pgtypes.CreateArrayTypeFromBaseType(elemType), nil
}
🟠 Legacy _anyelement definition invocation mismatch
  • What failed: The engine accepts legacy _anyelement-based definitions but cannot resolve compatible callable overloads later, producing runtime function not found failures.
  • Impact: Upgraded environments can retain or recreate legacy definitions that appear valid but fail when invoked, breaking existing SQL workloads. Users get late runtime failures instead of a clear upfront rejection or compatibility path.
  • Steps to reproduce:
    1. Create a legacy function signature that references _anyelement.
    2. Confirm the create statement succeeds.
    3. Invoke the function and observe function-not-found errors at runtime.
  • Stub / mock context: Authentication was intentionally bypassed by disabling SCRAM checks in server/authentication_scram.go so SQL engine behavior could be tested deterministically without login setup. No route mocking or API stubbing was applied to this test's type-resolution checks.
  • Code analysis: I examined the same underscore-based type fallback plus function overload resolution. _anyelement can be accepted as an implicit array-type lookup, but invocation relies on polymorphic compatibility and overload matching; if no compatible overload remains, the engine returns ErrFunctionDoesNotExist.
  • Why this is likely a bug: Production code accepts a legacy pseudo-type form at definition time but can reject compatible calls later, which is an internal consistency defect in type handling.

Relevant code:

core/typecollection/typecollection.go (lines 187-195)

// A name starting with "_" may be the implicit array type for a table's composite row
// type. Resolve the element type first (which handles the table lookup), then wrap it.
if strings.HasPrefix(typeName, "_") {
    elemType, err := pgs.GetType(ctx, id.NewType(name.SchemaName(), typeName[1:]))
    if err != nil || elemType == nil {
        return nil, err
    }
    return pgtypes.CreateArrayTypeFromBaseType(elemType), nil
}

server/functions/framework/compiled_function.go (lines 101-104)

// If we do not receive an overload, then the parameters given did not result in a valid match
if !overload.Valid() {
    c.stashedErr = ErrFunctionDoesNotExist.New(c.OverloadString(originalTypes))
    return c
}

server/functions/framework/compiled_function.go (lines 734-746)

// If one of the types is anyarray, then anyelement behaves as anynonarray, so we can convert them to anynonarray
for _, paramType := range paramTypes {
    if paramType.ID == pgtypes.AnyArray.ID {
        newParamTypes := make([]*pgtypes.DoltgresType, len(paramTypes))
        copy(newParamTypes, paramTypes)
        for i := range newParamTypes {
            if paramTypes[i].ID == pgtypes.AnyElement.ID {
                newParamTypes[i] = pgtypes.AnyNonArray
            }
        }
        paramTypes = newParamTypes
🟠 Array base-type resolution panics instead of returning graceful SQL errors
  • What failed: Instead of returning a graceful typed SQL error, the code reaches an explicit panic (cannot get base type) in the array base-type resolution path.
  • Impact: Queries involving unresolved custom array element types can terminate execution via panic instead of returning actionable SQL errors. This degrades reliability for type-resolution workflows and complicates recovery in runtime paths that depend on graceful error handling.
  • Steps to reproduce:
    1. Construct a query path that requires resolving a non-built-in array element type through ArrayBaseTypeCtx.
    2. Run it in a context where lookup does not yield a resolvable element type.
    3. Observe that execution reaches panic behavior instead of returning a controlled SQL error.
  • Stub / mock context: Real SCRAM authentication was bypassed by disabling the auth gate in server/authentication_scram.go. The scenario also intentionally exercised array resolution where context-backed type lookup could be unavailable to validate graceful-failure handling.
  • Code analysis: I inspected the array base-type resolver and initialization wiring. ArrayBaseTypeCtx still ends in an unconditional panic when built-in, logical, unresolved, and context lookups do not yield a type, and that behavior is directly reachable in unresolved/custom array scenarios.
  • Why this is likely a bug: The production resolver contains a direct panic branch for a recoverable type-resolution failure mode where a controlled SQL error is expected.

Relevant code:

server/types/type.go (lines 169-180)

if t.IsUnresolved && t.Elem != id.NullType {
	return NewUnresolvedDoltgresType(t.Elem.SchemaName(), t.Elem.TypeName())
}
if ctx != nil && t.Elem != id.NullType {
	if typeColl, err := GetTypesCollectionFromContext(ctx); err == nil && typeColl != nil {
		if elemType, err := typeColl.GetType(ctx, t.Elem); err == nil && elemType != nil {
			newElem := *elemType.WithAttTypMod(t.attTypMod)
			return &newElem
		}
	}
}
panic(fmt.Sprintf("cannot get base type from: %s", t.Name()))

core/init.go (lines 40-42)

pgtypes.GetTypesCollectionFromContext = func(ctx *sql.Context) (pgtypes.TypeCollection, error) {
	return GetTypesCollectionFromContext(ctx)
}
⚠️ Composite array result serialization crash
  • What failed: The function path should return a valid aggtype[] result, but execution hits errors (unbound variable "v1") and an alternate equivalent call path panics during result serialization instead of safely returning or surfacing a controlled SQL error.
  • Impact: Composite-array SQL function workflows are unreliable and can terminate query handling when this serialization path is reached. This breaks a core database execution path with no practical workaround for affected queries.
  • Steps to reproduce:
    1. Connect to local Doltgres as postgres.
    2. Create composite type aggtype(a integer, b integer, c text).
    3. Create a SQL function that accepts and returns aggtype[].
    4. Invoke the function with ARRAY[ROW(1,2,'hello')::aggtype] and fetch the result.
  • Stub / mock context: Authentication was bypassed by disabling SCRAM auth in server/authentication_scram.go so the SQL scenario could run deterministically in the local environment; no route interception or API response stubbing was applied.
  • Code analysis: I inspected server/types/utils.go, server/types/type.go, and server/doltgres_handler.go. The text-output row path calls Type.SQL(...), which delegates to sqlString; for array types, sqlString unconditionally type-asserts val to []any. In the reproduced failure mode, the value is already a string, so this assertion panics and explains the observed runtime crash.
  • Why this is likely a bug: The production code performs an unconditional val.([]any) cast in a path that can receive non-slice values, making panic behavior a direct code-level defect rather than test setup noise.

Relevant code:

server/types/utils.go (lines 146-150)

func sqlString(ctx *sql.Context, t *DoltgresType, val any) (string, error) {
    if t.IsArrayType() {
        baseType := t.ArrayBaseTypeCtx(ctx)
        return ArrToString(ctx, val.([]any), baseType, true)
    } else if t.ID == Bool.ID {

server/types/type.go (lines 917-923)

func (t *DoltgresType) SQL(ctx *sql.Context, dest []byte, v interface{}) (sqltypes.Value, error) {
    if v == nil {
        return sqltypes.NULL, nil
    }
    value, err := sqlString(ctx, t, v)
    if err != nil {

server/doltgres_handler.go (lines 796-800)

} else {
    val, err := s[i].Type.SQL(ctx, []byte{}, v) // We use []byte{} as there's a distinction between nil and empty
    if err != nil {
        return nil, err
    }
✅ Bugs Fixed (2)
Category Summary Screenshot
Polymorphic Unknown literal array input was coerced correctly and returned typed aggtype[] output. POLYMORPHIC-1
Polymorphic Implicit and explicit array literal calls produced identical results with stable coercion. POLYMORPHIC-2
✅ Verified Passing (6)
Category Summary Screenshot
Composite Composite array append function returned expected typed output. COMPOSITE-1
Composite Malformed quoted composite literal failed fast and parser recovered for later statements. COMPOSITE-5
Context Created vc4 domain and mood enum arrays, inserted literals, and verified round-trip values {ab,cd} and {sad,ok} via SQL. CONTEXT-1
Context Created mood enum and person_moods table, then verified 'ok' = ANY(moods) returns only row id 1. CONTEXT-4
Context Inserted {ab,NULL,cd} into vc4[] and verified element 2 is SQL NULL with array length 3. CONTEXT-5
Polymorphic Repeated malformed calls stayed bounded and a subsequent valid call succeeded. POLYMORPHIC-4
↪️ Inherited from Prior Run (7)

Tests that passed in the prior run. c3 judged them unaffected by the diff and did not retest.

Category Summary Screenshot
Composite Malformed literals failed immediately with explicit arity mismatch errors for too-few and too-many fields. N/A
Composite Malformed composite/array literals returned controlled parser errors and the backend remained responsive. N/A
Composite Mixed composite[] insert failed atomically on invalid input, then succeeded cleanly after correction. N/A
Context All malformed array literals failed with explicit parse/lexical errors and did not insert any data (COUNT(*)=0), demonstrating failure is critical and state remains consistent. N/A
Resolve Unqualified casts and ARRAY constructors resolved to the active schema's same-named type in each search_path context. N/A
Resolve Blank-schema array type fallback correctly resolved to the current schema (s2) for acctype[]. N/A
Resolve Inferred unresolved ARRAY casts matched explicit schema-qualified casts across schema-ordering contexts. N/A
⚠️ Additional Findings (1)

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

Origin Category Severity Summary Screenshot
🆕 New Composite ⚠️ High Consuming an accepted aggtype[] value triggered a panic instead of returning a controlled SQL error or result. COMPOSITE-6
⚠️ Composite array consumption triggers ArrayBaseType panic
  • What failed: The engine panics with cannot get base type from: _aggtype6 during downstream evaluation instead of producing a stable typed result or a controlled SQL error.
  • Impact: Queries that pass user-defined composite arrays through polymorphic paths can crash execution, breaking a core SQL workflow. This can terminate request handling for affected operations and has no reasonable SQL-side workaround.
  • Steps to reproduce:
    1. Create aggtype and define a function that appends ROW values into an aggtype[] array.
    2. Execute downstream calls that consume the produced array, such as consume_arr(make_arr()).
    3. Repeat with a direct array literal cast to aggtype[] and observe the panic path during evaluation.
  • Stub / mock context: SCRAM authentication was temporarily disabled in server/authentication_scram.go so local SQL execution could proceed without auth setup; no route interception or response stubbing was used for this case.
  • Code analysis: I inspected the polymorphic evaluation and array base-type resolution paths. CompiledFunction still uses ArrayBaseType() in polymorphic validation, which drops context, and ArrayBaseTypeCtx then panics when user-defined element type lookup cannot complete without context.
  • Why this is likely a bug: The production code takes a context-free array base-type path for polymorphic arrays and can deterministically hit an unconditional panic for user-defined array element types.

Relevant code:

server/functions/framework/compiled_function.go (lines 760-763)

baseExprType := exprTypes[i]
if baseExprType.IsArrayType() {
    baseExprType = baseExprType.ArrayBaseType()
}

server/types/type.go (lines 146-150)

func (t *DoltgresType) ArrayBaseType() *DoltgresType {
    // TODO: Remove this method and rename ArrayBaseTypeCtx(ctx)
    //       to ArrayBaseType(ctx) when all callers are migrated.
    return t.ArrayBaseTypeCtx(nil)
}

server/types/type.go (lines 169-180)

if t.IsUnresolved && t.Elem != id.NullType {
    return NewUnresolvedDoltgresType(t.Elem.SchemaName(), t.Elem.TypeName())
}
if ctx != nil && t.Elem != id.NullType {
    if typeColl, err := GetTypesCollectionFromContext(ctx); err == nil && typeColl != nil {
        if elemType, err := typeColl.GetType(ctx, t.Elem); err == nil && elemType != nil {
            newElem := *elemType.WithAttTypMod(t.attTypMod)
            return &newElem
        }
    }
}
panic(fmt.Sprintf("cannot get base type from: %s", t.Name()))

View Full Run


Tell us how we did: Give Ito Feedback

@fulghum fulghum merged commit 80c962c into main May 20, 2026
22 checks passed
@fulghum fulghum deleted the fulghum/bugfix branch May 20, 2026 19:06
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.

2 participants