Skip to content

2 AL/BC patterns: TableRelation field length and RecordRef.Open Temp parameter - #208

Merged
Jesper Schulz-Wedde (JesperSchulz) merged 6 commits into
microsoft:mainfrom
Curabis:community-contribution/tablerelation-length-and-recordref-temp
Oct 6, 2026
Merged

Jesper Schulz-Wedde (JesperSchulz) merged 6 commits into
microsoft:mainfrom
Curabis:community-contribution/tablerelation-length-and-recordref-temp

Conversation

@MichaelDieringer

@MichaelDieringer Michael Dieringer (MichaelDieringer) commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Two new articles about silent mismatches that compile without a diagnostic:

  • data-modeling/tablerelation-field-length-must-match-related-field.md: a referencing field shorter than its TableRelation target compiles cleanly. It fails at runtime ("The length of the string is N, but it must be less than or equal to M characters") once a real, longer related value is assigned or validated. The article states the exact rule from codeunit 134926 "Table Relation Test" (TableRelationTest.Codeunit.al:46, 48-49, 68-84):

    • At least one unconditional relation: exact length and same type.
    • Only conditional relations: at least as long.
    • Mixed Code/Text targets require Text.
    • Fields with ValidateTableRelation = false or TestTableRelation = false are skipped by the check. Microsoft uses longer filter/totaling fields deliberately, e.g. WarehouseSourceFilter."Variant Code Filter" (Code[100] related to Code[10]), AnalysisViewFilter."Dimension Value Filter" and JobTask.Totaling. So "longer" is only flagged when neither flag is false. "Shorter" is always a finding.

    Cross-linked to testing/table-relation-test-exclude-known-invalid-relations-via-event.md and data-modeling/transferfields-mirrored-fields-must-match-type-and-length.md.

  • testing/recordref-open-temp-parameter-defeats-real-table-checks.md: RecordRef.Open(TableNo, true) refers to an empty temporary instance. Any IsEmpty/Find*/Next/Get/Count existence or uniqueness check on it never sees persisted rows, and a generate-until-unused loop exits on its first pass. The positional, unnamed Boolean makes the call site look harmless.

    • Legitimate Temp = true uses are listed and excluded from the cue: a validation sandbox (ItemTempl.Table.al:1255), filter parsing/splitting via SetView/GetFilter (QltyFilterHelpers.Codeunit.al:942, IntegrationRecordSynch.Codeunit.al:221) and metadata reads.
    • The article defers to use-generateguid-for-unique-test-fixture-values.md for which LibraryUtility helper to call, and doesn't contradict it. LibraryUtility.Codeunit.al:288 vs :309 is cited as the Microsoft example.

Verification

  • Every fixture compiled with alc 30.0 against Base App 28.2 (plus Application Test Library 28.2 for the RecordRef samples).
    • TableRelation good/bad: no length diagnostic, which supports the "compiles cleanly" claim.
    • RecordRef good/bad: clean.
  • Every BCApps citation was re-opened at main 837ef8024.
  • Learn: TableRelation, ValidateTableRelation, TestTableRelation, AL0685, RecordRef.Open.
  • An earlier draft claimed that a where(...) filter does not make a relation conditional. That was removed as unverifiable, and the rule doesn't depend on it.

Wiring

  • al-data-modeling-review.md: scope line, Text[/ValidateTableRelation/TestTableRelation tokens, and a targeted check next to the existing TableRelation cue, including the carve-out.
  • al-testing-review.md: scope line, RecordRef/RecRef.Open/IsEmpty tokens, and a targeted check. It fires only on a literal true followed by an existence/uniqueness call on the same RecordRef with no Insert in between. Non-literal Temp, SetView/GetFilter-only use, and GenerateRandomCode itself are excluded.
  • Both slugs are registered in evaluation/review-fixtures.json.

Test plan

  • validate_frontmatter.py: 0 errors (2 pre-existing warnings in unrelated files)
  • Test-ReviewFixtures.ps1: 220 cases / 110 paired articles
  • Test-ReviewContract.ps1, Test-SkillIndex.ps1, Test-KnowledgeIndex.ps1, Test-KnowledgeRetrieval.ps1: pass
  • CLA: license/cla check passes (CURABIS ApS company agreement on file)
  • Domain owner review (microsoft/knowledge/data-modeling/, microsoft/knowledge/testing/)

🤖 Generated with Claude Code

- data-modeling: tablerelation-field-length-must-match-related-field
  (exact length for unconditional relations, at least the longest target
  when all relations are conditional; no compile-time diagnostic)
- testing: recordref-open-temp-parameter-defeats-real-table-checks
  (a temp-opened RecordRef is empty, so existence/uniqueness checks on it
  never see persisted rows; cross-links use-generateguid article)
- Wire both into the review leaves and register the fixture pairs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- tablerelation: fields with ValidateTableRelation/TestTableRelation = false
  are skipped by codeunit 134926 and may be longer (filter/totaling
  fields); shorter is still a finding. Add Code/Text type rule, BaseApp
  evidence, a ValidateTableRelation = false filter field to the good
  fixture, and the carve-out to the data-modeling cue.
- recordref: add FindLast/Next, relabel SplitLocalTableFilter as filter
  splitting, mark fixtures as test-library-style helpers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Table Relations Metadata mapping of where(...) filters to
Condition Field No. is not documented and could not be verified; the
rule does not depend on it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 89411ef60a108a5ef0af37db474a4df45971c708. The relation compatibility guidance matches the cited Table Relation Test implementation, including unconditional/all-conditional length distinctions and disabled-validation/filter-field exceptions. RecordRef.Open(..., true) correctly describes the empty temporary instance and is narrowly routed to persisted-row existence/uniqueness checks, preserving metadata/view-parsing/sandbox uses and existing LibraryUtility ownership. Both pairs have reachable deterministic positive/clean coverage. Exact-head local frontmatter, knowledge index/retrieval, skill/schema, review-contract, and changed-path fixture validation pass (220 cases). No merge-critical issue found. GitHub validation workflows are awaiting approval and should run before merge.

@JesperSchulz

Copy link
Copy Markdown
Contributor

Michael Dieringer (@MichaelDieringer), could you resolve the conflict? Looks good otherwise!



Insert-only conflicts in al-data-modeling-review.md (scope line, token list,
not-applicable list) and evaluation/review-fixtures.json: both sides kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MichaelDieringer

Copy link
Copy Markdown
Contributor Author

Resolved, thanks! Merged current main (insert-only overlaps with #203/#207 in al-data-modeling-review.md, both sides kept); all local validators pass.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 49b14834c49775af81c18161c50b97c28cec4b1a against the previously approved 89411ef60a108a5ef0af37db474a4df45971c708. This update merges main; all six approved knowledge/sample files are unchanged. The conflict resolutions preserve both TableRelation/RecordRef routing and main's Prices Including VAT applicability/tokens, and no main evaluation registration was lost. Exact-head local frontmatter, changed-path fixture preparation, contract, skill/schema, knowledge-index and retrieval checks pass. No merge-critical regression found. The substantive GitHub workflows are awaiting approval (action_required), rather than having passed, and should run before merge.

Resolve data-modeling applicability, token, and not-applicable conflicts by retaining TableRelation guidance alongside master-record insert/delete routing. Preserve approved canonical knowledge and samples, RecordRef exclusions, and all fixture registrations.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@JesperSchulz

Copy link
Copy Markdown
Contributor

The conflicts are resolved and validated at 1d140cb4fee0cf2962bd6c3fb20307832b910c43. Direct maintainer push still fails HTTP 403 on the Curabis fork, so the resolution is available as Curabis/BCQuality#5 targeting this PR's source branch.

Please merge the sync PR with Create a merge commit to preserve main's ancestry and clear the conflicts. The approved knowledge/samples are unchanged, both sets of routing are retained, and all local fixture/contract/index checks pass. Once synchronized, we intend to merge #208.

@MichaelDieringer

Copy link
Copy Markdown
Contributor Author

Merged Curabis#5 with a merge commit, thanks! I've also enabled "Allow edits by maintainers" on #208 and #214, so you should be able to push directly from now on.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed synchronized head 9409fa5352480c74827200a068ddfd3b9963c7d6. Its entire tree is identical to validated conflict-resolution commit 1d140cb4fee0cf2962bd6c3fb20307832b910c43. All six approved knowledge/sample files remain unchanged; additive routing resolutions preserve both the TableRelation surface and main's master-record insert/delete guidance, and all main fixture registrations are retained. Exact-head local frontmatter, changed-path fixture preparation (248 cases), review contract, skill/schema and index/retrieval checks pass. The original PR is now non-conflicting and no merge-critical regression remains. The four substantive GitHub validation workflows are awaiting approval (action_required) and should run before merge.

@JesperSchulz
Jesper Schulz-Wedde (JesperSchulz) merged commit 018e627 into microsoft:main Oct 6, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants