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

Add migration script to remove size limits from namespaces, dataset n… #1925

Merged
merged 3 commits into from
Mar 29, 2022

Conversation

collado-mike
Copy link
Collaborator

Problem

Several columns in the database are restricted to 255 characters, perhaps without strong justification. Several of these cause problems, including namespaces, in #1758, as well as the connection_url when storing some Snowflake tables (the connection URLs contain a lot of extra parameters which aren't always scrubbed before sending the OpenLineage events). If there isn't a strong reason for restricting these columns, the default restriction on VARCHARs will work just as well with no performance penalty.

Closes: #1758

Solution

Remove the size restriction on various VARCHAR types

Checklist

  • You've signed-off your work
  • Your changes are accompanied by tests (if relevant)
  • Your change contains a small diff and is self-contained
  • You've updated any relevant documentation (if relevant)
  • You've updated the CHANGELOG.md with details about your change under the "Unreleased" section (if relevant, depending on the change, this may not be necessary)
  • You've versioned your .sql database schema migration according to Flyway's naming convention (if relevant)

…ames, and source connection urls

Signed-off-by: Michael Collado <collado.mike@gmail.com>
@collado-mike collado-mike requested a review from wslulciuc March 24, 2022 23:21
@codecov
Copy link

codecov bot commented Mar 24, 2022

Codecov Report

Merging #1925 (4445d70) into main (26d1a68) will not change coverage.
The diff coverage is n/a.

❗ Current head 4445d70 differs from pull request most recent head b3c9fc2. Consider uploading reports for the commit b3c9fc2 to get more accurate results

@@            Coverage Diff            @@
##               main    #1925   +/-   ##
=========================================
  Coverage     77.90%   77.90%           
  Complexity      937      937           
=========================================
  Files           193      193           
  Lines          5218     5218           
  Branches        418      418           
=========================================
  Hits           4065     4065           
  Misses          706      706           
  Partials        447      447           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Collaborator

@fm100 fm100 left a comment

Choose a reason for hiding this comment

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

👍

Comment on lines +1 to +10
ALTER TABLE dataset_versions ALTER COLUMN namespace_name TYPE VARCHAR;
ALTER TABLE dataset_versions ALTER COLUMN dataset_name TYPE VARCHAR;
ALTER TABLE datasets ALTER COLUMN name TYPE VARCHAR;
ALTER TABLE datasets ALTER COLUMN physical_name TYPE VARCHAR;
ALTER TABLE datasets ALTER COLUMN source_name TYPE VARCHAR;
ALTER TABLE namespaces ALTER COLUMN name TYPE VARCHAR;
ALTER TABLE runs ALTER COLUMN external_id TYPE VARCHAR;
ALTER TABLE sources ALTER COLUMN name TYPE VARCHAR;
ALTER TABLE sources ALTER COLUMN connection_url TYPE VARCHAR;
ALTER TABLE tags ALTER COLUMN name TYPE VARCHAR;
Copy link
Collaborator

Choose a reason for hiding this comment

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

So these are to change from VARCHAR(256) to VARCHAR, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Internal server error 500 - field name too long
3 participants