Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MLv2] Handle Object.isFrozen() legacy columns as input #39032

Merged
merged 2 commits into from
Feb 22, 2024

Conversation

bshepherdson
Copy link
Contributor

cljs.core/hash mutates vanilla JS arrays to attach a UID to them for
hashing purposes. Before this change, given a DatasetColumn with a
frozen field_ref array, we would use it directly on the
:metadata/column, and (hash column) would try to mutate the frozen
array and throw.

This clones the array while converting the legacy column to MLv2 form,
so there's no issue with mutating it.

Fixes the closure_uid_123456789 issue.

`cljs.core/hash` mutates vanilla JS arrays to attach a UID to them for
hashing purposes. Before this change, given a `DatasetColumn` with a
frozen `field_ref` array, we would use it directly on the
`:metadata/column`, and `(hash column)` would try to mutate the frozen
array and throw.

This clones the array while converting the legacy column to MLv2 form,
so there's no issue with mutating it.

Fixes the `closure_uid_123456789` issue.
Copy link
Contributor Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@metabase-bot metabase-bot bot added the .Team/QueryProcessor :hammer_and_wrench: label Feb 21, 2024
Copy link

replay-io bot commented Feb 21, 2024

Status Complete ↗︎
Commit 523d154
Results
⚠️ 3 Flaky
2313 Passed

@bshepherdson bshepherdson added this to the 0.49 milestone Feb 21, 2024
@bshepherdson bshepherdson added the backport Automatically create PR on current release branch on merge label Feb 21, 2024
Copy link
Contributor

@uladzimirdev uladzimirdev left a comment

Choose a reason for hiding this comment

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

:shipit:

@bshepherdson bshepherdson merged commit fb815da into master Feb 22, 2024
107 checks passed
@bshepherdson bshepherdson deleted the mblib-handle-frozen-columns branch February 22, 2024 14:44
bshepherdson added a commit that referenced this pull request Mar 26, 2024
…39032)"

Co-authored-by: Alexander Polyankin <alexander.polyankin@metabase.com>
bshepherdson added a commit that referenced this pull request Mar 26, 2024
…39032)"

Co-authored-by: Alexander Polyankin <alexander.polyankin@metabase.com>
bshepherdson added a commit that referenced this pull request Mar 26, 2024
…39032)"

Co-authored-by: Alexander Polyankin <alexander.polyankin@metabase.com>
bshepherdson added a commit that referenced this pull request Mar 26, 2024
…39032)" (#39057)

Co-authored-by: Braden Shepherdson <braden@metabase.com>
Co-authored-by: Alexander Polyankin <alexander.polyankin@metabase.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge .Team/QueryProcessor :hammer_and_wrench:
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants