Skip to content

structaccess: reject lossy numeric conversions in Set - #6922

Open
denik wants to merge 7 commits into
mainfrom
denik/structaccess-lossy-numeric-conversion
Open

denik wants to merge 7 commits into
mainfrom
denik/structaccess-lossy-numeric-conversion

Conversation

@denik

@denik denik commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Changes

structaccess.Set converted between numeric kinds with reflect.Convert, which silently truncates floats (1.9 → 1) and wraps out-of-range integers (int64(1<<40) into int32 → 0, -1 into uint). A lossy reference could deploy a wrong number.

Now conversions to integer kinds return an error (e.g. cannot set 1.9 to int: precision loss, value 1099511627776 overflows int32), following the same rules as libs/dyn/convert normalizeInt. Integral floats (2.0 → 2) still work. Conversions to float kinds stay allowed even when they round. Applies to direct and pointer targets; non-numeric conversions are unchanged.

Tests

  • Table test in libs/structs/structaccess covering every rule (allowed and rejected, direct and pointer targets), and a structvar.ResolveRef test showing the error surfaces.
  • Acceptance test bundle/resource_deps/bad_ref_float_to_int: a float field referenced from an int field now fails planning. Existing resource_deps and variables goldens are unchanged.
  • Acceptance test bundle/variables/float_to_int_field records the existing ${var.x} behaviour for a float variable in an int field (validate warns and drops the value); unchanged by this PR.

This pull request and its description were written by Isaac.

denik and others added 2 commits October 5, 2026 11:42
convertValue fell back to reflect.Convert for numeric kinds, which truncates
floats and wraps out-of-range integers. Check for precision loss and overflow,
matching the rules in libs/dyn/convert.

Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 32b9682

Run: 37318229737

Env ✅​pass 🙈​skip Time
✅​ aws linux 276 18 6:47
✅​ aws windows 278 16 4:27
✅​ azure linux 275 18 6:51
✅​ azure windows 277 16 3:28
✅​ gcp linux 276 18 5:59
✅​ gcp windows 278 16 3:30
Top 6 slowest tests (at least 2 minutes):
duration env testname
4:01 azure linux TestAccept
4:00 gcp linux TestAccept
3:57 aws linux TestAccept
3:30 aws windows TestAccept
3:28 gcp windows TestAccept
3:20 azure windows TestAccept

… test

Co-authored-by: Isaac <no-reply@databricks.com>
@github-actions github-actions Bot added the DABs DABs related issues label Oct 5, 2026
denik and others added 4 commits October 5, 2026 13:24
…lossy float to float

Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
@denik
denik marked this pull request as ready for review October 5, 2026 13:44
@denik
denik requested review from a team as code owners October 5, 2026 13:44
@denik
denik requested a review from andrewnester October 5, 2026 13:44

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DABs DABs related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants