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

Dont allow protected column names on import #14294

Merged
merged 11 commits into from
Aug 2, 2024

Conversation

adrinr
Copy link
Collaborator

@adrinr adrinr commented Jul 31, 2024

Description

We have some protected column names, such as _id and tableId, that can not be used in the schema. When exporting some csv we are exporting some of these columns, and when creating a new table via CSV upload allowed creating these columns as part of the schema.
This PR ensures that this is not the case, both on the API level and on the client side

Screen.Recording.2024-08-01.at.11.02.43.mov

Launchcontrol

Handle protected column names on table imports

@github-actions github-actions bot added firestorm Data/Infra/Revenue Team size/m labels Jul 31, 2024
Base automatically changed from fix/reimporting_rows to master August 1, 2024 09:03
@adrinr adrinr marked this pull request as ready for review August 1, 2024 09:04
@adrinr adrinr requested a review from a team as a code owner August 1, 2024 09:04
@adrinr adrinr requested review from mike12345567 and removed request for a team August 1, 2024 09:04
@adrinr adrinr enabled auto-merge August 1, 2024 10:36
@adrinr adrinr requested a review from samwho August 1, 2024 11:22

isInternal &&
it.each(
isInternal ? PROTECTED_INTERNAL_COLUMNS : PROTECTED_EXTERNAL_COLUMNS
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this needed? Doesn't the outer condition make this redundant?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Amended here: #14302

@adrinr adrinr merged commit e36cd17 into master Aug 2, 2024
12 checks passed
@adrinr adrinr deleted the fix/dont-allow-protected-column-names-on-import branch August 2, 2024 08:46
@github-actions github-actions bot locked and limited conversation to collaborators Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
firestorm Data/Infra/Revenue Team size/m
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants