Resolve SQL standard type aliases by name in the parser type table - #3233
Open
reltuk wants to merge 1 commit into
Open
Resolve SQL standard type aliases by name in the parser type table#3233reltuk wants to merge 1 commit into
reltuk wants to merge 1 commit into
Conversation
A schema-qualified type name such as `pg_catalog.boolean` reaches the PL/pgSQL interpreter as text rather than as parser input, so it can only be resolved by looking the name up in the parser's type name table, which listed almost none of the SQL standard aliases. This adds them, and normalizes whitespace within a name so that the multi-word spellings match however they happen to be written. The interpreter now maps a name it finds to the spelling the type is registered under, which also fixes `bytea`, `bpchar` and `json` resolving to a different type or to none.
reltuk
enabled auto-merge
August 31, 2026 12:17
Contributor
|
Contributor
|
@reltuk DOLT
|
|
SummaryThe run covers database function behavior for qualified type names across normal values, aliases, whitespace normalization, arrays, length and precision limits, schema isolation, invalid inputs, error recovery, and existing cast behavior. Broad happy-path and edge-case coverage shows most type handling remains healthy, but array handling and constrained type declarations have important correctness gaps. Not safe to merge yet — this PR introduces a high-severity failure in qualified array declarations and a medium-severity failure in qualified type modifiers, affecting whether functions can be called successfully and whether declared constraints are honored. These are attributable behavior regressions in core type resolution, not unrelated caveats. Tests run by Ito
Tip Reply with @itoqa to send us feedback on this test run. |
![itoqa[bot]](https://avatars.githubusercontent.com/in/2622614?s=60&v=4){kind=small}
There was a problem hiding this comment.
Qualified type limits are lost
What failed: Calling the function with the qualified character type failed instead of preserving its length limit. The decimal case returned a value, but the qualified character varying(5) case reported that the full text was not a type.
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
- Severity: Medium
- Impact: Functions that declare a schema-qualified character type with a length limit fail instead of running correctly. Users may need to avoid the qualified form, and the affected declaration cannot enforce its intended limit.
- Steps to Reproduce:
- Create a PL/pgSQL function with a local variable declared as pg_catalog.decimal(8,2), pg_catalog.character varying(5), or pg_catalog.bit varying(4), and assign a value that fits the modifier.
- Call each function and inspect its returned value and type metadata.
- Compare the results with the declaration's precision, character length, or bit length; the qualified character varying(5) case fails with a type-does-not-exist error.
- Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
- Code Analysis: server/plpgsql/statements.go:115-124 copies variable.Type into InterpreterOperation.PrimaryData without separating the base type from its modifier, so a declaration such as pg_catalog.character varying(5) reaches execution as one string. In server/plpgsql/interpreter_logic.go:129-160, the OpCode_Declare handler removes quotes, splits the schema from the remaining text, and calls types.TypeForNonKeywordTypeName(elementName) only to map an alias. That lookup receives the modifier-bearing text and cannot turn character varying(5) into the base alias character varying. The handler then calls typeCollection.GetType(ctx, id.NewType(schemaName, typeName)) at line 160 using the unchanged modifier-bearing name when alias lookup misses, producing the observed type-does-not-exist error. Even where a base alias is found, the code replaces typeName with arrayPrefix + typ.PGName() at lines 154-157, which has no typmod component, so the declaration's precision, character length, or bit length is not represented in the catalog lookup. The smallest practical fix is to parse the type name into its base name and modifier before alias canonicalization, resolve the base name, and pass the parsed modifier through the declaration/type construction path rather than treating the entire declaration type as a catalog name.
- Why this is likely a bug: The expected behavior is that a qualified alias and its modifier behave like the corresponding PostgreSQL type declaration. The local SQL evidence shows a concrete failure for pg_catalog.character varying(5), while source inspection explains it without relying on the unavailable browser endpoint: the resolver performs a string lookup on a modifier-bearing name and has no representation for the modifier after canonicalization. This is directly within the PR's changed PL/pgSQL mapping branch, not a test-only or browser problem. A targeted fix that separates the base alias from its typmod and preserves the typmod during type resolution should address the failure without changing unrelated schema lookup behavior.
Relevant code
server/plpgsql/statements.go:115-124
for _, variable := range stmt.Variables {
op := InterpreterOperation{
OpCode: OpCode_Declare,
PrimaryData: variable.Type,
Target: variable.Name,
}
var val any
if variable.Default != "" {
op.SecondaryData = []string{variable.Default}
val = variable.Default
}server/plpgsql/interpreter_logic.go:137-160
typeName := operation.PrimaryData
typeName = strings.ReplaceAll(typeName, `"`, "")
schemaName := "pg_catalog"
if strings.Contains(typeName, ".") {
parts := strings.Split(typeName, ".")
schemaName = parts[0]
typeName = parts[1]
if schemaName == "pg_catalog" {
arrayPrefix := ""
elementName := typeName
if strings.HasPrefix(typeName, "_") {
arrayPrefix, elementName = "_", typeName[1:]
}
typ, ok, _ := types.TypeForNonKeywordTypeName(elementName)
if ok && typ != nil {
typeName = arrayPrefix + typ.PGName()
}
}postgres/parser/types/types.go:2522-2539
func normalizeTypeName(name string) string {
if !strings.ContainsAny(name, " \t\n\r\f\v") {
return name
}
return typeNameWhitespace.ReplaceAllString(strings.TrimSpace(name), " ")
}
func TypeForNonKeywordTypeName(name string) (*T, bool, int) {
name = normalizeTypeName(name)
t, ok := typNameLiterals[name]
if ok {
return t, ok, 0
}Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.
**Medium severity — Qualified type limits are lost**
**What failed:** Calling the function with the qualified character type failed instead of preserving its length limit. The decimal case returned a value, but the qualified character varying(5) case reported that the full text was not a type.
- **Impact:** Functions that declare a schema-qualified character type with a length limit fail instead of running correctly. Users may need to avoid the qualified form, and the affected declaration cannot enforce its intended limit.
- **Steps to reproduce:**
1. Create a PL/pgSQL function with a local variable declared as pg_catalog.decimal(8,2), pg_catalog.character varying(5), or pg_catalog.bit varying(4), and assign a value that fits the modifier.
2. Call each function and inspect its returned value and type metadata.
3. Compare the results with the declaration's precision, character length, or bit length; the qualified character varying(5) case fails with a type-does-not-exist error.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** server/plpgsql/statements.go:115-124 copies variable.Type into InterpreterOperation.PrimaryData without separating the base type from its modifier, so a declaration such as pg_catalog.character varying(5) reaches execution as one string. In server/plpgsql/interpreter_logic.go:129-160, the OpCode_Declare handler removes quotes, splits the schema from the remaining text, and calls types.TypeForNonKeywordTypeName(elementName) only to map an alias. That lookup receives the modifier-bearing text and cannot turn character varying(5) into the base alias character varying. The handler then calls typeCollection.GetType(ctx, id.NewType(schemaName, typeName)) at line 160 using the unchanged modifier-bearing name when alias lookup misses, producing the observed type-does-not-exist error. Even where a base alias is found, the code replaces typeName with arrayPrefix + typ.PGName() at lines 154-157, which has no typmod component, so the declaration's precision, character length, or bit length is not represented in the catalog lookup. The smallest practical fix is to parse the type name into its base name and modifier before alias canonicalization, resolve the base name, and pass the parsed modifier through the declaration/type construction path rather than treating the entire declaration type as a catalog name.
- **Why this is likely a bug:** The expected behavior is that a qualified alias and its modifier behave like the corresponding PostgreSQL type declaration. The local SQL evidence shows a concrete failure for pg_catalog.character varying(5), while source inspection explains it without relying on the unavailable browser endpoint: the resolver performs a string lookup on a modifier-bearing name and has no representation for the modifier after canonicalization. This is directly within the PR's changed PL/pgSQL mapping branch, not a test-only or browser problem. A targeted fix that separates the base alias from its typmod and preserves the typmod during type resolution should address the failure without changing unrelated schema lookup behavior.
**Relevant code:**
`server/plpgsql/statements.go:115-124`
~~~go
for _, variable := range stmt.Variables {
op := InterpreterOperation{
OpCode: OpCode_Declare,
PrimaryData: variable.Type,
Target: variable.Name,
}
var val any
if variable.Default != "" {
op.SecondaryData = []string{variable.Default}
val = variable.Default
}
~~~
`server/plpgsql/interpreter_logic.go:137-160`
~~~go
typeName := operation.PrimaryData
typeName = strings.ReplaceAll(typeName, `"`, "")
schemaName := "pg_catalog"
if strings.Contains(typeName, ".") {
parts := strings.Split(typeName, ".")
schemaName = parts[0]
typeName = parts[1]
if schemaName == "pg_catalog" {
arrayPrefix := ""
elementName := typeName
if strings.HasPrefix(typeName, "_") {
arrayPrefix, elementName = "_", typeName[1:]
}
typ, ok, _ := types.TypeForNonKeywordTypeName(elementName)
if ok && typ != nil {
typeName = arrayPrefix + typ.PGName()
}
}
~~~
`postgres/parser/types/types.go:2522-2539`
~~~go
func normalizeTypeName(name string) string {
if !strings.ContainsAny(name, " \t\n\r\f\v") {
return name
}
return typeNameWhitespace.ReplaceAllString(strings.TrimSpace(name), " ")
}
func TypeForNonKeywordTypeName(name string) (*T, bool, int) {
name = normalizeTypeName(name)
t, ok := typNameLiterals[name]
if ok {
return t, ok, 0
}
~~~
There was a problem hiding this comment.
Qualified array declarations fail when called
What failed: The functions were accepted, but invoking the qualified array functions produced a malformed array literal error. The expected array values, element types, and array metadata were not returned.
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
- Severity: High
- Impact: Users who define functions with schema-qualified array types cannot call those functions successfully. Their array values and type metadata do not return as expected.
- Steps to Reproduce:
- Connect to the local Doltgres server as postgres.
- Create a PL/pgSQL function with a local variable declared as pg_catalog.boolean[] and initialize it with a representative array value.
- Repeat with pg_catalog.character varying[] and pg_catalog.double precision[], then create a comparable function using pg_catalog._int4.
- Call each function and check whether the array value and array type are returned.
- Observe the malformed array literal error instead of a successful array round-trip.
- Stub / mock content: Local SCRAM authentication was disabled so the test could connect to the local Doltgres server. No application mocks, route interceptions, or test-data bypasses were used.
- Code Analysis: In server/plpgsql/statements.go:115-129, each PL/pgSQL declaration is emitted as an OpCode_Declare whose PrimaryData contains the type text. In server/plpgsql/interpreter_logic.go:129-160, the declaration text is split into schemaName and typeName before TypeCollection lookup. The PR-modified branch at lines 140-158 handles an array only when typeName starts with '_' (lines 149-153), strips that prefix for alias lookup, and restores it with typ.PGName() (line 156). It never parses a SQL [] suffix. Therefore a declaration such as pg_catalog.boolean[] keeps the bracket form while alias lookup expects a base element name and the TypeCollection lookup cannot establish the correct registered array type. The subsequent default path at lines 167-189 calls resolvedType.IoInput with the declaration's default text; with the wrong or incomplete array resolution, the ARRAY[...] expression is treated as an invalid array literal, matching the recorded error. The smallest practical fix is to detect and remove the [] suffix before alias lookup, resolve the normalized element alias, then construct the underscore-prefixed registered array name before calling GetType; the internal pg_catalog._int4 spelling must continue to work as it does today.
- Why this is likely a bug: The test exercises ordinary PostgreSQL array declarations, not an artificial fault or an unavailable service. The local server accepted all declarations and then failed consistently when the resulting functions were called, while the source path independently shows that SQL [] syntax is not converted to the underscore-prefixed array lookup form used by the type collection. PostgreSQL users expect boolean[], character varying[], and double precision[] to preserve array shape and values; a malformed literal at invocation prevents that core function workflow. The PR's changed mapping branch is the smallest practical repair surface because it already owns schema-qualified alias and array-name normalization.
Relevant code
server/plpgsql/interpreter_logic.go:129-160
case OpCode_Declare: ... typeName := operation.PrimaryData ... if strings.Contains(typeName, ".") { ... if schemaName == "pg_catalog" { arrayPrefix := ""; elementName := typeName; if strings.HasPrefix(typeName, "_") { arrayPrefix, elementName = "_", typeName[1:] }; typ, ok, _ := types.TypeForNonKeywordTypeName(elementName); if ok && typ != nil { typeName = arrayPrefix + typ.PGName() } } } ... resolvedType, err := typeCollection.GetType(ctx, id.NewType(schemaName, typeName))server/plpgsql/statements.go:115-129
for _, variable := range stmt.Variables { op := InterpreterOperation{OpCode: OpCode_Declare, PrimaryData: variable.Type, Target: variable.Name}; ... *ops = append(*ops, op); stack.NewVariableWithValue(variable.Name, nil, val) }server/types/type.go:143-154
func NewUnresolvedArrayDoltgresType(sch, elemName string) *DoltgresType { return &DoltgresType{ID: id.NewType(sch, "_"+elemName), IsUnresolved: true, TypCategory: TypeCategory_ArrayTypes, Elem: &DoltgresType{ID: id.NewType(sch, elemName), IsUnresolved: true}, ...} }Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.
**High severity — Qualified array declarations fail when called**
**What failed:** The functions were accepted, but invoking the qualified array functions produced a malformed array literal error. The expected array values, element types, and array metadata were not returned.
- **Impact:** Users who define functions with schema-qualified array types cannot call those functions successfully. Their array values and type metadata do not return as expected.
- **Steps to reproduce:**
1. Connect to the local Doltgres server as postgres.
2. Create a PL/pgSQL function with a local variable declared as pg_catalog.boolean[] and initialize it with a representative array value.
3. Repeat with pg_catalog.character varying[] and pg_catalog.double precision[], then create a comparable function using pg_catalog._int4.
4. Call each function and check whether the array value and array type are returned.
5. Observe the malformed array literal error instead of a successful array round-trip.
- **Stub / mock content:** Local SCRAM authentication was disabled so the test could connect to the local Doltgres server. No application mocks, route interceptions, or test-data bypasses were used.
- **Code analysis:** In server/plpgsql/statements.go:115-129, each PL/pgSQL declaration is emitted as an OpCode_Declare whose PrimaryData contains the type text. In server/plpgsql/interpreter_logic.go:129-160, the declaration text is split into schemaName and typeName before TypeCollection lookup. The PR-modified branch at lines 140-158 handles an array only when typeName starts with '_' (lines 149-153), strips that prefix for alias lookup, and restores it with typ.PGName() (line 156). It never parses a SQL [] suffix. Therefore a declaration such as pg_catalog.boolean[] keeps the bracket form while alias lookup expects a base element name and the TypeCollection lookup cannot establish the correct registered array type. The subsequent default path at lines 167-189 calls resolvedType.IoInput with the declaration's default text; with the wrong or incomplete array resolution, the ARRAY[...] expression is treated as an invalid array literal, matching the recorded error. The smallest practical fix is to detect and remove the [] suffix before alias lookup, resolve the normalized element alias, then construct the underscore-prefixed registered array name before calling GetType; the internal pg_catalog._int4 spelling must continue to work as it does today.
- **Why this is likely a bug:** The test exercises ordinary PostgreSQL array declarations, not an artificial fault or an unavailable service. The local server accepted all declarations and then failed consistently when the resulting functions were called, while the source path independently shows that SQL [] syntax is not converted to the underscore-prefixed array lookup form used by the type collection. PostgreSQL users expect boolean[], character varying[], and double precision[] to preserve array shape and values; a malformed literal at invocation prevents that core function workflow. The PR's changed mapping branch is the smallest practical repair surface because it already owns schema-qualified alias and array-name normalization.
**Relevant code:**
`server/plpgsql/interpreter_logic.go:129-160`
~~~go
case OpCode_Declare: ... typeName := operation.PrimaryData ... if strings.Contains(typeName, ".") { ... if schemaName == "pg_catalog" { arrayPrefix := ""; elementName := typeName; if strings.HasPrefix(typeName, "_") { arrayPrefix, elementName = "_", typeName[1:] }; typ, ok, _ := types.TypeForNonKeywordTypeName(elementName); if ok && typ != nil { typeName = arrayPrefix + typ.PGName() } } } ... resolvedType, err := typeCollection.GetType(ctx, id.NewType(schemaName, typeName))
~~~
`server/plpgsql/statements.go:115-129`
~~~go
for _, variable := range stmt.Variables { op := InterpreterOperation{OpCode: OpCode_Declare, PrimaryData: variable.Type, Target: variable.Name}; ... *ops = append(*ops, op); stack.NewVariableWithValue(variable.Name, nil, val) }
~~~
`server/types/type.go:143-154`
~~~go
func NewUnresolvedArrayDoltgresType(sch, elemName string) *DoltgresType { return &DoltgresType{ID: id.NewType(sch, "_"+elemName), IsUnresolved: true, TypCategory: TypeCategory_ArrayTypes, Elem: &DoltgresType{ID: id.NewType(sch, elemName), IsUnresolved: true}, ...} }
~~~
Contributor
Author
|
I believe these two itoqa call outs are actual issues with the plpgsql implementation, but they are not directly caused by this PR. This PR is improving support for some types of existing qualified type names, which currently fail to parse and/or resolve at compile time. It still doesn't handle all cases of parameterized types or array types, but it does improve things somewhat from the status quo. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A schema-qualified type name such as
pg_catalog.booleanreaches the PL/pgSQL interpreter as text rather than as parser input, so it can only be resolved by looking the name up in the parser's type name table, which listed almost none of the SQL standard aliases. This adds them, and normalizes whitespace within a name so that the multi-word spellings match however they happen to be written. The interpreter now maps a name it finds to the spelling the type is registered under, which also fixesbytea,bpcharandjsonresolving to a different type or to none.