RET_CHECK when casting a range variable to a struct through a no-op cast
- Dominant language
- C++
- Stars
- 2.6k
- Forks
- 260
- PR merge metrics
- No merged PRs in 30d
Description
The analyzer fails with an internal `RET_CHECK` instead of resolving, when a range variable is cast to a struct type and an intervening cast is eliminated as a no-op.
Reproduced on `fd972655db97deac02f0696ea652a390209b794b`.
Also reproduced with the official GoogleSQL 2026.7.2 `execute_query` Linux
binary using `--catalog=sample --mode=analyze`; it prints the same error.
## Repro
```sql
SELECT CAST(CAST(kv AS STRUCT) AS STRUCT)
FROM KeyValue kv
```
Actual:
```
INTERNAL: GOOGLESQL_RET_CHECK failure (googlesql/analyzer/function_resolver.cc:1139)
Cannot obtain the AST expressions for field arguments of struct constructor:
CastExpression [12-55]
PathExpression [17-19]
Identifier(kv) [17-19]
...
```
Expected: the statement resolves, taking the same struct `Cast` that the bare
`CAST(kv AS STRUCT)` already takes today.
The inner cast has to be a **no-op** — its target type must equal the range
variable's own type — and the outer cast has to target a struct. If the inner
cast changes the type, or is absent, the statement resolves normally.
The same query returns `An internal error occurred and the request could not be
completed. Error: 80038528` from the BigQuery service (observed 2026-08-05), so
this is not specific to a local build configuration.
## Mechanism
`FunctionResolver::AddCastOrConvertLiteral`
(`googlesql/analyzer/function_resolver.cc:1362`) decomposes a struct cast field
by field, guarded at `:1398`:
```cpp
if (target_type->IsStruct() &&
argument->get()->node_kind() == RESOLVED_MAKE_STRUCT &&
ast_location->node_kind() != AST_PATH_EXPRESSION) {
```
and calls `ExtractStructFieldLocations` (`:1076`) immediately inside that branch,
at `:1413`.
The `AST_PATH_EXPRESSION` exclusion is there for a good reason, stated in the
comment above it: a range variable resolves to a `ResolvedMakeStruct` that no
written struct constructor stands behind, so there are no field expressions to
extract, and `ExtractStructFieldLocations`' `default:` arm is a
`GOOGLESQL_RET_CHECK_FAIL` (`:1138`).
**The two disagree about which AST node to look at.**
`ExtractStructFieldLocations` first walks down through gratuitous wrappers
(`:1081`):
```cpp
// Skip through gratuitous casts in the AST so that we can get the field
// argument locations.
const ASTNode* cast_free_ast_location = ast_location;
while (cast_free_ast_location != nullptr) {
if (cast_free_ast_location->node_kind() == AST_CAST_EXPRESSION) {
...
} else if (cast_free_ast_location->node_kind() == AST_NAMED_ARGUMENT) {
...
```
and only then switches on the node kind. The guard at `:1400` does not skip
anything — it tests `ast_location` directly.
So for the repro:
1. The inner `CAST` is a no-op, and `ResolveCastWithResolvedArgument`
(`resolver_expr.cc:7612`) drops it — `IsCastNoop` (`:7593`) returns true when
the source and target types and annotations match, and unless
`preserve_unnecessary_cast()` is set the function returns without building a
`ResolvedCast` (`:7620`).
2. The argument reaching the outer cast is therefore the range variable's
`ResolvedMakeStruct`, exactly as in the bare-path-expression case.
3. But `ast_location` is now the *inner* `ASTCastExpression`, not the
`ASTPathExpression`, so the guard's exclusion does not fire and the branch is
entered.
4. `ExtractStructFieldLocations` skips back through that same cast, arrives at
`AST_PATH_EXPRESSION`, finds no constructor, and hits its `RET_CHECK_FAIL`.
The immediate defect is inconsistent AST normalization: in this repro the guard
is one wrapper too shallow, and the elision in step 1 is what puts that wrapper
in between. More generally, the branch is selected with a partial negative test
(`not AST_PATH_EXPRESSION`), while the extractor requires a positive struct-
constructor shape.
## Suggested fix
The immediate fix is to normalize `ast_location` through the same cast and
named-argument wrappers before applying the path-expression exclusion. That
closes this repro and other wrapped-path variants: the normalized node is
`AST_PATH_EXPRESSION`, so resolution falls through to the whole-struct `Cast`.
Normalization alone does not make the guard and extractor agree on every
possible input. The guard would still admit any normalized non-path node, while
`ExtractStructFieldLocations` accepts exactly
`AST_STRUCT_CONSTRUCTOR_WITH_PARENS`, `AST_STRUCT_CONSTRUCTOR_WITH_KEYWORD`,
`AST_BRACED_CONSTRUCTOR` and `AST_STRUCT_BRACED_CONSTRUCTOR`. Any other operand
that resolves to a `ResolvedMakeStruct` would still reach the extractor's
`default:` arm.
A more robust fix is to combine recognition and extraction into one helper, for
example `TryExtractStructFieldLocations`, that:
1. unwraps gratuitous casts and named arguments;
2. returns field locations for one of the supported struct constructors; and
3. returns no locations for every other node kind.
`AddCastOrConvertLiteral` should use the field-by-field path only when that
helper returns locations; otherwise it should fall through to the whole-struct
`Cast`. This shares both wrapper normalization and the accepted-node set by
construction, while retaining `RET_CHECK`s for impossible field-count
mismatches after a constructor has been recognized.
A positive `IsSupportedStructConstructor` test on the normalized node would
also close all currently unsupported node kinds, but duplicating the constructor
list next to the extractor could allow the two to drift again.
## Suggested test cases
```sql
-- currently RET_CHECKs; should resolve
SELECT CAST(CAST(kv AS STRUCT) AS STRUCT)
FROM KeyValue kv
-- already resolves; guards the fix against regressing the bare case
SELECT CAST(kv AS STRUCT) FROM KeyValue kv
-- already resolves; the field-by-field path must stay field-by-field
SELECT CAST(STRUCT(1 AS a, 'x' AS b) AS STRUCT)
-- already resolves; non-literal fields ensure this actually exercises the
-- extractor's wrapper-skipping path and remains field-by-field
SELECT CAST(CAST((Key, Value) AS STRUCT) AS STRUCT)
FROM KeyValue
```
Contributor guide
Research direction
Start in googlesql/analyzer/function_resolver.cc at AddCastOrConvertLiteral and ExtractStructFieldLocations, then trace no-op cast removal in resolver_expr.cc at ResolveCastWithResolvedArgument. Reproduce the query with the analyzer or execute_query, align wrapper handling between the guard and extractor, and verify all four supplied SQL cases resolve without regressing field-by-field casts.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, sql
- Domain
- compilers, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100