finos / finos/legend-engine

Semi-structured: union-sourced and Enumeration to-many properties are still bound to a stringified array

Open
#5,193 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
112
Forks
260
Avg merge
1d 13h
Merged PRs (30d)
96

Description

### Summary

#5105 made a `[*]` semi-structured path bound to a to-many property fan out to one row per
element, closing #5099. Two shapes are excluded by its gate and are expected to fall back to
the pre-#5105 behaviour: one row holding the array's text, with no error at compile or run
time. Neither is covered by a test today, here or in #5105 — the exclusions were written to
keep known-bad inputs away from the flatten, not because the fallback was measured.

### The gate

`processRelationalPropertyMapping` in `pureToSQLQuery.pure`:

```pure
let isImplicitArrayExplosion = !$property.multiplicity->hasToOneUpperBound()
&& $state.disableAutoFlatten != true
&& $OrigrelationalPropertyMappings->at(0).transformer->isEmpty()
&& !$newCurrentTreeNode.alias.relationalElement->toOne()->isUnionOrNestedUnion()
&& $mappingImpl->isArrayValuedRelationalOperation();
```

### The two exclusions

**1. Union-sourced array mappings.** `processRelationalOperationElementOfPropertyMapping`
early-exits for Union and embedded sources with a deliberately bogus column type, so the
navigation handed to the flatten would be the wrong operand. Excluded rather than flattened
incorrectly.

**2. To-many `Enumeration` properties.** An enum transformer is read from the *pre*-pushdown
mapping list, because `buildPossibleEnumMappingPushDown` clears `transformer` and rewrites the
operation to a `case` dyna. A to-many enum bound to a `[*]` path is therefore excluded. This
exclusion is currently defensive only: the transpose tests that would exercise it are
`<>` and do not run, so nothing proves it fires.

Possibly also: nesting an implicit explosion inside another one. #5105's two-level wildcard
test (`divisions[*].teams[*].label`) passes, so the ordinary nested-path case is covered; what
is untested is an array-valued property reached *through* an already-fanned-out property. That
shape has not been characterised, so it may be fine.

### Why this is worth tracking

Both exclusions leave the exact silent wrong answer #5099 described. The modeller declares a
to-many, spot-checks an array-shaped value, and ships a property that is really a single
string. #5105 removed that window for the common case. These keep it open for two others.

First step for whoever picks this up: write the two models and record what actually happens.

Related: #5099 (closed by #5105), #5114 (primitive to-many operations never reach `array_*`).

Contributor guide

Open the contributing guide

Research direction

Start in pureToSQLQuery.pure, especially processRelationalPropertyMapping and processRelationalOperationElementOfPropertyMapping. Add models covering union-sourced arrays and to-many Enumeration properties, then run the relevant transpose tests and record whether each shape compiles and returns one row per element or a stringified array. Done means the observed behavior is characterized and the remaining incorrect fallback is addressed with coverage.

Written by the indexing model from the issue text.

Assessment

Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
50/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.