Skip to content
This repository was archived by the owner on May 2, 2023. It is now read-only.

Conversation

@dlawin
Copy link
Contributor

@dlawin dlawin commented Feb 6, 2023

Similar change for postgres as:
#13
#16

@erezsh
Copy link
Contributor

erezsh commented Feb 13, 2023

When does postgres support 3 part table paths? Can you include a test for this?

@dlawin
Copy link
Contributor Author

dlawin commented Feb 17, 2023

When does postgres support 3 part table paths? Can you include a test for this?

All of the providers with 3 part support so far allow you to use 1, 2, or 3 part ids. When using 2 or 3 parts, you are overriding the default "schema" and/or "database". Although these namespaces are referred to differently sometimes

Copy link
Contributor

@erezsh erezsh left a comment

Choose a reason for hiding this comment

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

Looks good enough!

Just one small change if you please - no need to create a new decorator. Instead just use @test_each_database along with @skipIf if the db is within the set of dbs. Because in a future refactor, we're going to use something like db.max_path_len >= 3.

@dlawin
Copy link
Contributor Author

dlawin commented Feb 23, 2023

Looks good enough!

Just one small change if you please - no need to create a new decorator. Instead just use @test_each_database along with @skipIf if the db is within the set of dbs. Because in a future refactor, we're going to use something like db.max_path_len >= 3.

Wasn't quite sure on how to implement with the decorator, but I added an if statement instead

@dlawin dlawin requested a review from erezsh February 23, 2023 02:21
@dlawin dlawin force-pushed the postgres_3_part_id branch from 69cc4f9 to 04ab11b Compare February 23, 2023 18:26
@dlawin dlawin requested a review from erezsh February 23, 2023 18:27
@erezsh erezsh merged commit 40159a3 into master Feb 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants