Skip to content

Fix view auto-migrate with canonical names#4985

Open
JasonAtClockwork wants to merge 2 commits intomasterfrom
jlarabie/ts-view-remove-add-issue
Open

Fix view auto-migrate with canonical names#4985
JasonAtClockwork wants to merge 2 commits intomasterfrom
jlarabie/ts-view-remove-add-issue

Conversation

@JasonAtClockwork
Copy link
Copy Markdown
Contributor

Description of Changes

Closes: #4842

Fixes the auto-migration for views where the exported name differs from the accessor/source name in the validation.

The bug looked like the columns weren't matching and it turned out that the columns were keyed to the accessor name then attempted to match up against the canonical name. That caused the auto-migration to think the columns changed and triggered a remove/add for the view.

API and ABI breaking changes

None intended

Expected complexity level and risk

2 - This is small and targeted but I'm not familiar with this part of the codebase

Testing

  • Rebuilt the sample from the issue in TypeScript and confirmed the issue and the repair
  • Built a mirror in Rust to confirm it was an issue for all languages
  • Ran local tests

@joshua-spacetime
Copy link
Copy Markdown
Collaborator

I think the main issue is that ViewValidator uses TableValidator, but name resolution is different between the two because views and tables are in different namespaces. So I think a better way to fix this is to pass a name resolver to TableValidator so that we can pass the correct name resolver to all the child defs (which is what I did in the latest commit).

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.

False-positive JS view migration: identical view is planned as RemoveView + AddView

2 participants