Skip to content

When databasename is empty it should not be added to prefix.#1605

Merged
rohkhann merged 13 commits intomainfrom
rohkhann/FixTablePrefix
Jul 28, 2023
Merged

When databasename is empty it should not be added to prefix.#1605
rohkhann merged 13 commits intomainfrom
rohkhann/FixTablePrefix

Conversation

@rohkhann
Copy link
Copy Markdown
Contributor

@rohkhann rohkhann commented Jul 24, 2023

Why make this change?

What is this change?

  • Summary of how your changes work to give reviewers context of your intent.
    Added check to see if databasename is null or empty then we will not add it to the table prefix.

How was this tested?

  • Integration Tests
    existing integration tests test this path of code
  • Unit Tests
    Added new test called checkTablePrefix. Chose to go with internalsvisibleto route as the main goal was to test gettableprefix method and ROI on moqing out components of the public method initializeasync to do the test was low.

@aaronpowell
Copy link
Copy Markdown
Contributor

Are we able to add tests for this?

Comment thread src/Core/Services/MetadataProviders/SqlMetadataProvider.cs Outdated
@rohkhann rohkhann closed this Jul 24, 2023
@rohkhann rohkhann reopened this Jul 24, 2023
Comment thread src/Core/Services/MetadataProviders/SqlMetadataProvider.cs
Copy link
Copy Markdown
Collaborator

@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, left few nits.

Comment thread src/Core/Services/MetadataProviders/SqlMetadataProvider.cs Outdated
Comment thread src/Core/Services/MetadataProviders/SqlMetadataProvider.cs Outdated
Comment thread src/Service.Tests/Unittests/SqlMetadataProviderUnitTests.cs Outdated
Comment thread src/Core/Services/MetadataProviders/SqlMetadataProvider.cs
Copy link
Copy Markdown
Contributor

@ayush3797 ayush3797 left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment thread src/Service.Tests/Unittests/SqlMetadataProviderUnitTests.cs Outdated
Comment thread src/Service.Tests/Unittests/SqlMetadataProviderUnitTests.cs Outdated
Comment thread src/Core/Services/MetadataProviders/SqlMetadataProvider.cs
@rohkhann rohkhann enabled auto-merge (squash) July 28, 2023 21:32
@rohkhann rohkhann merged commit 60e38a2 into main Jul 28, 2023
@rohkhann rohkhann deleted the rohkhann/FixTablePrefix branch July 28, 2023 21:47
@Aniruddh25 Aniruddh25 linked an issue Aug 21, 2023 that may be closed by this pull request
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.

[Bug] When database name is empty it generates wrong sql query

5 participants