Closed liamcervante closed 1 year ago
Thanks! This makes sense to me. It is true that this is probably gonna be a bit expensive, but better to be correct for now and then we can look for opportunities to optimize later if we discover this is causing a real problem.
The original issue is surfaced in terraform as https://github.com/hashicorp/terraform/issues/32109
The root cause is that go-cty is not stripping dynamic information out of null values recursively, it only checks if the value itself is dynamic but not for example if it is a collection and has a dynamic element type. The new function behaves basically like
WithoutOptionalAttributesDeep
except it's taking away dynamic types where possible. If both the in and out types are dynamic then the function has no choice but to also return dynamic.This approach is the simplest from an architecture point of view, once we decide we are returning a null value then we recurse through the type and replace any dynamic types we would return. However, it's a bit inefficient as it recalculates things like unified types in objects and tuples when this work was already done. To fix that we could take the short circuit out entirely and have every conversion function handle null and unknown themselves, but that's a lot of changes needed and this is quite simple to implement and I don't think too negatively performant.