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

[Cherry pick] Table name collision bug fix (#1990) #2065

Merged
merged 2 commits into from
Mar 4, 2024

Conversation

seantleonard
Copy link
Contributor

@seantleonard seantleonard commented Mar 1, 2024

Cherry picks to release/0.11 from main

Why make this change?

Closes #1908

What is this change?

We were previously filling the schema information for the entity data set but only using the table name. We add in the schema name to deconflict between two tables with the same name in different schemas. This means that we retrieve information from the entity data set by using schema and tablename when schema is available, and just the table name otherwise.

The reason that we do not need to worry about collisions at the schema and table name level (eg. where we have the same schema and table names within different databases) is because this entity data set is a part of the SqlMetadataProvider, and we have a SqlMetadataProvider for each database (not database type but actual DBs).

How was this tested?

A regression test with a collision between table names in different schemas was added. We also update the unit test for SqlMetadatProvider.

## Why make this change?

Closes #1908

## What is this change?

We were previously filling the schema information for the entity data
set but only using the table name. We add in the schema name to
deconflict between two tables with the same name in different schemas.
This means that we retrieve information from the entity data set by
using schema and tablename when schema is available, and just the table
name otherwise.

The reason that we do not need to worry about collisions at the schema
and table name level (eg. where we have the same schema and table names
within different databases) is because this entity data set is a part of
the SqlMetadataProvider, and we have a SqlMetadataProvider for each
database (not database type but actual DBs).

## How was this tested?

A regression test with a collision between table names in different
schemas was added. We also update the unit test for SqlMetadatProvider.

## Sample Request(s)

see #1908
@seantleonard
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@Aniruddh25
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Contributor

@Aniruddh25 Aniruddh25 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 for the port

@seantleonard seantleonard enabled auto-merge (squash) March 2, 2024 00:22
@Aniruddh25
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@Aniruddh25
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@seantleonard seantleonard merged commit c9a2125 into release/0.11 Mar 4, 2024
9 checks passed
@seantleonard seantleonard deleted the dev/sean/tableconflit_cherrypick011 branch March 4, 2024 17:00
@seantleonard seantleonard added the 🍒Cherrypick Cherry-picking another commit/PR label May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒Cherrypick Cherry-picking another commit/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants