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

Add support for unlogged tables in PostgreSQL #6046

Merged
merged 8 commits into from Jun 7, 2023

Conversation

f-lombardo
Copy link
Contributor

@f-lombardo f-lombardo commented May 24, 2023

Q A
Type bug/feature/improvement
Feature see #6044

Summary

Add support for creating unlogged PostgreSQL tables:
https://www.postgresql.org/docs/current/sql-createtable.html

I added a boolean unlogged option to default_table_options doctrine configuration
https://www.doctrine-project.org/projects/doctrine-bundle/en/latest/configuration.html
For example:

doctrine:
    dbal:
        default_table_options:
            unlogged: true

@derrabus derrabus changed the base branch from 3.6.x to 3.7.x May 25, 2023 10:25
@derrabus derrabus changed the base branch from 3.7.x to 3.6.x May 25, 2023 10:25
@derrabus derrabus changed the base branch from 3.6.x to 3.7.x May 25, 2023 10:26
@derrabus
Copy link
Member

Thank you. As always, we need functional tests that prove that your change actually works. Assertions on generated SQL are never sufficient.

In this case, the following scenarios must be covered:

  • Create a table with that new option.
  • Introspect logged and unlogged tables.
  • Compare a logged with an unlogged table and vice versa.
  • What happens if I use that option on a database that does not support it?

@derrabus
Copy link
Member

Since this is a new feature and 3.6.x is closed for features, I'm targeting the 3.7.x branch now. Please rebase your PR onto that branch to make sure you don't run into merge conflicts.

@f-lombardo
Copy link
Contributor Author

* What happens if I use that option on a database that does not support it?

I think nothing should happen, since the option shouldn't be used by any other db driver

@SenseException
Copy link
Member

UNLOGGED was introduced in 9.1, if I believe what the docs say. At least this isn't breaking between versions supported by DBAL.

What's the use case that made you create the PR for adding this new feature?

@f-lombardo
Copy link
Contributor Author

f-lombardo commented May 26, 2023

What's the use case that made you create the PR for adding this new feature?

I have many Behat tests that read/write data on PostgreSQL DBs and I'd like to run them faster. As I'm using test data, I'm not concerned about possible data corruption in case of crashes.

tests/Functional/Driver/PDO/PgSQL/DBAL6044Test.php Outdated Show resolved Hide resolved
tests/Functional/Driver/PDO/PgSQL/DBAL6044Test.php Outdated Show resolved Hide resolved
tests/Functional/Driver/PDO/PgSQL/DBAL6044Test.php Outdated Show resolved Hide resolved
@derrabus derrabus added this to the 3.7.0 milestone Jun 7, 2023
@derrabus derrabus merged commit f97f825 into doctrine:3.7.x Jun 7, 2023
70 checks passed
@derrabus
Copy link
Member

derrabus commented Jun 7, 2023

Thank you!

@f-lombardo
Copy link
Contributor Author

Thank you!

It was a pleasure!
Do you think we should also edit documentation about this new feature?

@derrabus
Copy link
Member

derrabus commented Jun 7, 2023

Sure, go ahead!

@f-lombardo
Copy link
Contributor Author

Sure, go ahead!

I'm looking at the docs folder and I'm a bit confused. What docs do yo think I should edit?

@derrabus
Copy link
Member

derrabus commented Jun 7, 2023

I'm a bit confused

Why's that?

@f-lombardo
Copy link
Contributor Author

I don't understand where to put this new information, since I haven't found any part of the documentation regarding the already existing table options for MySQL.

@derrabus
Copy link
Member

derrabus commented Jun 8, 2023

There a chapter on schema representation. This is where this piece of information fits best, I'd say. But note that…

This chapter is far from being completely documented.

derrabus added a commit that referenced this pull request Jul 15, 2023
<!-- Fill in the relevant information below to help triage your pull
request. -->

|      Q       |   A
|------------- | -----------
| Type         | improvement
| Fixed issues | Add docs for #6044 

#### Summary

Add docs for #6044 / #6046

---------

Co-authored-by: Alexander M. Turek <me@derrabus.de>
@f-lombardo
Copy link
Contributor Author

Hi @derrabus , do you already have a planned date for a release containing this feature?

Thanks.

@derrabus
Copy link
Member

No. But I realize that there are already some features waiting to be released, so I don't want to wait too long either. That being said, you can use the feature already by using a 3.7.x-dev snapshot.

cgknx pushed a commit to cgknx/dbal that referenced this pull request Aug 30, 2023
<!-- Fill in the relevant information below to help triage your pull
request. -->

|      Q       |   A
|------------- | -----------
| Type         | improvement
| Fixed issues | Add docs for doctrine#6044 

#### Summary

Add docs for doctrine#6044 / doctrine#6046

---------

Co-authored-by: Alexander M. Turek <me@derrabus.de>
@f-lombardo
Copy link
Contributor Author

Any news about release dates for this update?

@greg0ire
Copy link
Member

greg0ire commented Oct 5, 2023

Yes.

2023-10-05_16-03

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants