Skip to content

fix(ENGKNOW-3577): store actual link data name in create cache#124

Merged
gmagnu merged 8 commits into
mainfrom
ENGKNOW-3577-gor-create-with-write-link-fails
Jun 18, 2026
Merged

fix(ENGKNOW-3577): store actual link data name in create cache#124
gmagnu merged 8 commits into
mainfrom
ENGKNOW-3577-gor-create-with-write-link-fails

Conversation

@gmagnu

@gmagnu gmagnu commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • A GOR create #x# = ... | write -link X ; nor [#x#] failed: the create cache stored a data-file name that did not match the file actually written.
  • Root cause: LinkFileUtil.inferDataFileNameFromLinkFile is non-idempotent (random suffix), and the name was derived twice — at plan time in BaseScriptExecutionEngine.resolveCache (cached) and again at exec time in ForkWrite (actual write) — yielding two different names.
  • Fix: in resolveCache, for a link-inferred write (no positional filename + -link present), inject the resolved data-file name into the executed query so ForkWrite reuses it instead of re-deriving. commandToExecute is now read after getExplicitWrite so the injected name reaches execution. No change to LinkFileUtil.

Test Plan

  • New regression test gorsat.UTestGorWrite.testCreateWriteLinkCacheName — fails before the fix (cache points at non-existent file), passes after.
  • Full gorsat.UTestGorWrite (60) green.
  • gorsat.Script.* (34) green — covers the commandToExecute read relocation.

Ticket: ENGKNOW-3577

🤖 Generated with Claude Code

gmagnu and others added 5 commits June 18, 2026 13:40
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A create ending in `write -link X` derived the data file name twice
(plan time in resolveCache, exec time in ForkWrite); the name is
non-idempotent (random suffix), so the cached name did not match the
written file and referencing the create failed. resolveCache now injects
the resolved name into the executed write so ForkWrite reuses it.

The injection only takes effect because commandToExecute is now read
after getExplicitWrite runs, instead of before, so the executed command
reflects the query mutation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown

Junit Tests - Summary

4 705 tests  +4   4 528 ✅ +5   18m 15s ⏱️ +19s
  473 suites ±0     177 💤 ±0 
  473 files   ±0       0 ❌  - 1 

Results for commit 463528f. ± Comparison against base commit 7e962ab.

♻️ This comment has been updated with latest results.

gmagnu and others added 3 commits June 18, 2026 15:06
…gor/parallel

Adds create-wrapped tests referencing [#test#] for pgor, parallel and
partgor. Confirms getCachePath reuses the resolved cache path (no
re-derivation), so the create-cache-name mismatch does not occur for
these macros. partgor test asserts cache-name correctness only: partgor
does not currently propagate the -link option (no link file written) -
a separate pre-existing defect noted as a follow-up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PartGor built its dictionary command but never appended -link/-linkmeta,
unlike PGor and Parallel, so `partgor ... | write -link X` wrote no link
file. Append the link options (from create.query) to the partgor
dictionary command, mirroring Parallel. The partgor regression test now
asserts the link file is written and resolves, like the pgor/parallel
variants.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@gumson gumson left a comment

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.

LGTM!

@gmagnu gmagnu merged commit be39e72 into main Jun 18, 2026
14 checks passed
@gmagnu gmagnu deleted the ENGKNOW-3577-gor-create-with-write-link-fails branch June 18, 2026 16:04
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