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

Support temporary schemas in get_schema() #107

Merged
merged 32 commits into from
Feb 20, 2024

Conversation

RasmusSkytte
Copy link
Contributor

@RasmusSkytte RasmusSkytte commented Feb 9, 2024

Intent

This PR adds the temporary argument to get_schema(), get_catalog() to allow it to get the temporary schema/catalog for the connection.

In addition, get_schema() can now infer the schema from tbl_dbi objects (by using id() internally).

Tests are also added and improved

The CI workflows for Microsoft SQL Server has also been updated over on AEF-DDF.
Before, the both the permanent and temporary databases were called "tempdb" which masked a few issues in the identification functions. The new workflows uses "master" and "tempdb" respectively, so we also fix some previously undetected issues.

The documentation of the S3 methods get_schema() and get_catalog() was adjusted to try to figure out the best way of documenting the functions. Discussion available on #69.

Approach

  • The temporary parameter was added to some get_schema() methods.

  • id() was used to infer schema from tbl_dbi

  • Bugfixes made for table_exists() and get_tables(), specifically for MSSQL

  • Better error message added for id.tbl_dbi in case table has been deleted.

Known issues

Tests for PostgreSQL are showing as failing.
This is caused by an update to the Workflows so that previously undetected errors are now being explicitly shown.
The source of the error is being fixed in #98

Checklist

  • The PR passes all local unit tests
  • I have documented any new features introduced
  • If the PR adds a new feature, please add an entry in NEWS.md
  • A reviewer is assigned to this PR

@RasmusSkytte RasmusSkytte self-assigned this Feb 9, 2024
@RasmusSkytte RasmusSkytte added the enhancement New feature or request label Feb 9, 2024
@RasmusSkytte RasmusSkytte force-pushed the feature/get_temporary_schemas branch 3 times, most recently from 19f813c to 4ac1fb5 Compare February 12, 2024 11:02
@RasmusSkytte RasmusSkytte marked this pull request as ready for review February 12, 2024 11:34
@RasmusSkytte RasmusSkytte requested review from a team, kaare-gr and LasseEngboChr and removed request for a team February 12, 2024 11:34
@RasmusSkytte RasmusSkytte added this to the v0.4 milestone Feb 12, 2024
@RasmusSkytte RasmusSkytte mentioned this pull request Feb 13, 2024
5 tasks
@RasmusSkytte RasmusSkytte mentioned this pull request Feb 14, 2024
6 tasks
@RasmusSkytte RasmusSkytte merged commit 56054a0 into main Feb 20, 2024
23 of 24 checks passed
@RasmusSkytte RasmusSkytte deleted the feature/get_temporary_schemas branch February 20, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants