google / google/googlesql

RET_CHECK on a nested parameterized type when FEATURE_CREATE_TABLE_FIELD_ANNOTATIONS is off

Open
#173 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
2.6k
Forks
260
PR merge metrics
No merged PRs in 30d

Description

## Summary

With `FEATURE_PARAMETERIZED_TYPES` enabled and
`FEATURE_CREATE_TABLE_FIELD_ANNOTATIONS` disabled, resolving a `CREATE TABLE`
column whose **nested** type has parameters returns an `INTERNAL` status from a
reachable `GOOGLESQL_RET_CHECK`:

```sql
CREATE TABLE t (a ARRAY)
CREATE TABLE t (a STRUCT)
```

This is a reference-implementation analyzer/diagnostic bug: valid SQL input and
a supported feature combination should not reach an internal invariant check.
The two language features have no declared dependency.

Originally reproduced at `fd972655db97deac02f0696ea652a390209b794b`.
Re-verified with the official `execute_query` binary from release `2026.7.2`;
the relevant resolver code is unchanged through
`1f8aa333f4d6353cd3a64471fc83121df72df3f7`.

## Reproduction

```sh
execute_query --mode=analyze \
--enabled_language_features='NONE,+PARAMETERIZED_TYPES' \
'CREATE TABLE t (a ARRAY)'
```

Actual:

```text
ERROR: INTERNAL: GOOGLESQL_RET_CHECK failure (googlesql/analyzer/resolver_stmt.cc:2337) annotations != nullptr
```

The same result occurs for `STRUCT`, `RANGE`, and `MAP` child schemas, for
example:

```sql
CREATE TABLE t (a STRUCT)
CREATE TABLE t (a RANGE) -- also enable RANGE_TYPE and TIMESTAMP_PRECISION
CREATE TABLE t (a MAP) -- also enable MAP_TYPE
CREATE TABLE t (a MAP) -- also enable MAP_TYPE
```

Controls:

| Features/input | Analyzer outcome |
|---|---|
| Parameterized types on, field annotations off, nested `STRING(5)` | `INTERNAL` |
| Both features on, nested `STRING(5)` | Resolution succeeds and the resolved AST contains nested type parameters |
| Parameterized types on, field annotations off, top-level `STRING(5)` | Resolution succeeds |
| Parameterized types off, `STRING(5)` anywhere | Clean `Parameterized types are not supported` SQL error |

For the two successful controls, `execute_query` prints the resolved AST and
may then report an unrelated `UNIMPLEMENTED` while installing the analyzed
`CREATE TABLE` in its in-memory catalog because that path does not consume
non-default `ResolvedColumnDefinition::annotations`. That later tool error is
not this analyzer failure.

There is an analogous collation case, independent of parameterized types:

```sh
execute_query --mode=analyze \
--enabled_language_features='NONE,+COLLATION_SUPPORT' \
"CREATE TABLE t (a ARRAY)"
```

This reaches the same `annotations != nullptr` check. With collation support
disabled it instead returns the clean `COLLATE is not supported` SQL error.

## Root cause

`Resolver::ResolveColumnSchema` uses one boolean for two different decisions:

```cpp
const bool enable_nested_annotations =
annotations != nullptr &&
language().LanguageFeatureEnabled(FEATURE_CREATE_TABLE_FIELD_ANNOTATIONS);
```

See [`resolver_stmt.cc:2088-2090`](https://github.com/google/googlesql/blob/1f8aa333f4d6353cd3a64471fc83121df72df3f7/googlesql/analyzer/resolver_stmt.cc#L2088-L2090).
When the field-annotations feature is off, recursive calls for array/range
elements, struct fields, and map keys/values receive `annotations = nullptr`
([element path](https://github.com/google/googlesql/blob/1f8aa333f4d6353cd3a64471fc83121df72df3f7/googlesql/analyzer/resolver_stmt.cc#L2009-L2025),
[struct path](https://github.com/google/googlesql/blob/1f8aa333f4d6353cd3a64471fc83121df72df3f7/googlesql/analyzer/resolver_stmt.cc#L2152-L2169),
[map paths](https://github.com/google/googlesql/blob/1f8aa333f4d6353cd3a64471fc83121df72df3f7/googlesql/analyzer/resolver_stmt.cc#L2191-L2208)).

The nested call nevertheless resolves type parameters whenever
`FEATURE_PARAMETERIZED_TYPES` is enabled. Type parameters are not carried by
the resulting `Type*`; `CREATE TABLE` stores them in
`ResolvedColumnAnnotations`. The final annotation construction therefore sees
non-empty `resolved_type_parameters` but no output sink:

```cpp
if ((resolved_collation != nullptr) || not_null ||
!resolved_column_options.empty() || !child_annotation_list.empty() ||
!resolved_type_parameters.IsEmpty()) {
GOOGLESQL_RET_CHECK(annotations != nullptr);
// ...
}
```

See [`resolver_stmt.cc:2331-2342`](https://github.com/google/googlesql/blob/1f8aa333f4d6353cd3a64471fc83121df72df3f7/googlesql/analyzer/resolver_stmt.cc#L2331-L2342).
`resolved_collation` can violate the same invariant for the collation feature
combination above.

The existing feature guards only reject type parameters or `COLLATE` when their
own features are disabled. They do not handle successful resolution with a null
annotation sink.

An important complication is that `annotations == nullptr` is also used near
the start of the function to reject nested attributes and `OPTIONS` when
`FEATURE_CREATE_TABLE_FIELD_ANNOTATIONS` is off. Simply passing a non-null sink
to every child would accidentally accept those constructs.

## Expected behavior and possible fixes

At minimum, this input should produce a user-facing SQL error rather than an
`INTERNAL` status. The more composable interpretation is to accept nested type
parameters when `FEATURE_PARAMETERIZED_TYPES` is enabled, regardless of the
field-annotations feature, because the parameterized-types feature admits them
and the resolved AST can represent them.

1. **Preferred semantic fix: decouple collection from permission.** Recurse
with child annotation sinks whenever the parent has a sink, so independently
enabled type modifiers can be preserved. Separately check whether nested
attributes and `OPTIONS` are allowed by
`FEATURE_CREATE_TABLE_FIELD_ANNOTATIONS`; do not use sink nullness as that
policy check. Nested collation should likewise follow an explicit feature
policy rather than being accepted or rejected as a side effect of sink
allocation.
2. **Minimal diagnostic fix:** when a nested call has no sink, reject an AST
type-parameter list (and the analogous `COLLATE`) with a SQL error before it
can reach annotation construction. This preserves the current storage/policy
behavior but makes the independently enabled features non-composable for
nested `CREATE TABLE` types.

Silently discarding the resolved parameters is not a valid fix because it loses
schema semantics. The final `RET_CHECK` can remain as an invariant after all
reachable input paths either allocate a sink or return a SQL error.

## Regression tests

Cover nested parameters in `ARRAY`, `STRUCT`, `RANGE`, and both `MAP` child
positions with parameterized types on and field annotations off. Also retain
controls for:

- top-level parameters;
- the same nested types with field annotations on;
- parameterized types disabled (clean SQL error);
- nested `NOT NULL` and `OPTIONS` with field annotations off (still rejected);
- the analogous nested collation feature matrix.

Contributor guide

Open the contributing guide

Research direction

Start in googlesql/analyzer/resolver_stmt.cc at ResolveColumnSchema, especially the child recursion and annotation check around lines 2088-2090 and 2331-2342. Run the provided execute_query reproductions, then inspect existing analyzer regression-test patterns. Done means nested ARRAY, STRUCT, RANGE, and MAP parameters no longer produce INTERNAL with field annotations off, while the listed feature and NOT NULL/OPTIONS controls retain their expected results.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, sql
Domain
backend-api-design, databases, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
64/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.