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

feat(ingest): Redash add parse_table_names_from_sql feature and multiple refactor #3267

Merged
merged 4 commits into from
Oct 6, 2021

Conversation

taufiqibrahim
Copy link
Contributor

@taufiqibrahim taufiqibrahim commented Sep 20, 2021

  • Add feature parse table names from SQL
  • Adding QualifiedNameParser and several extended dataclasses for parsing table name into fully qualified name based on database type. As an example, table name orders in Postgres database should be normalized into databaseName.public.orders if only table name part given
  • Some small refactors to improve

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable)

@taufiqibrahim taufiqibrahim marked this pull request as ready for review September 20, 2021 11:42
Copy link
Contributor

@shirshanka shirshanka left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @taufiqibrahim

@shirshanka shirshanka merged commit ed38168 into datahub-project:master Oct 6, 2021
jgiannuzzi pushed a commit to G-Research/datahub that referenced this pull request Oct 6, 2021
swaroopjagadish pushed a commit to swaroopjagadish/datahub that referenced this pull request Oct 7, 2021
@taufiqibrahim taufiqibrahim deleted the redash branch October 12, 2021 17: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