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: add temp_schema option for table_diff #3311

Merged
merged 3 commits into from
Nov 5, 2024

Conversation

Kayrnt
Copy link
Contributor

@Kayrnt Kayrnt commented Oct 30, 2024

So far, table_diff will create a temp table in sqlmesh_temp schema.
However in some environments, the user might want to use an existing schema for that because he either don't want to create and manage another schema or he doesn't have the rights to do so.

The PR introduces a CLI parameter --temp-schema <catalog>.<schema> or --temp-schema <schema> to write the temp tables inside that catalog and/or schema.

To do list:

  • Check that works as expected
  • Ensure it works with API Endpoint
  • Add related documentation

@Kayrnt Kayrnt force-pushed the table_diff_dataset branch 2 times, most recently from b48d5d2 to 87001ce Compare November 1, 2024 16:36
@@ -34,7 +35,7 @@ def get_table_diff(
show=False,
)
_schema_diff = diff.schema_diff()
_row_diff = diff.row_diff()
_row_diff = diff.row_diff(temp_schema=temp_schema)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I'm not breaking something here, I guess it's fine.

@Kayrnt Kayrnt marked this pull request as ready for review November 1, 2024 17:14
Copy link
Contributor

@georgesittas georgesittas left a comment

Choose a reason for hiding this comment

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

Seems good, thanks for the PR @Kayrnt 👍

@Kayrnt
Copy link
Contributor Author

Kayrnt commented Nov 4, 2024

I forgot to delete a small part I added in the doc (following a Copilot suggestion 🙈).
I clean it up, I think it should be good to go 👍

@tobymao tobymao merged commit 47a9e67 into TobikoData:main Nov 5, 2024
20 checks passed
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.

3 participants