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 postgresql 14 #266

Closed
wants to merge 14 commits into from
Closed

Conversation

Leo-XM-Zeng
Copy link
Contributor

No description provided.

@JelteF JelteF changed the title supprot postgresql 14 Support postgresql 14 Oct 9, 2024
@Leo-XM-Zeng
Copy link
Contributor Author

@JelteF Excuse me
There is an error that postgresql14 does not support the following SQL
image
Would like to ask, how to modify here

@JelteF
Copy link
Collaborator

JelteF commented Oct 9, 2024

I think the easiest is to change those specific tests to a python test and skip them for PG14. i.e. create a new file test/pycheck/temporary_table_test.py.

To be clear we won't have time to review this in detail or merge this before the 0.1.0 release. But given the small number of necessary changes, I think this can definitely be targeted for 0.2.0.

Thank you for your contribution

@JelteF JelteF added the enhancement New feature or request label Oct 9, 2024
@JelteF JelteF added this to the 0.2.0 milestone Oct 9, 2024
@Leo-XM-Zeng
Copy link
Contributor Author

I think the easiest is to change those specific tests to a python test and skip them for PG14. i.e. create a new file test/pycheck/temporary_table_test.py.

To be clear we won't have time to review this in detail or merge this before the 0.1.0 release. But given the small number of necessary changes, I think this can definitely be targeted for 0.2.0.

Thank you for your contribution

Understood. Thank you for your guidance

from .utils import Cursor, PG_MAJOR_VERSION, eprint


def test_temporary_table(cur: Cursor):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def test_temporary_table(cur: Cursor):
def test_temporary_table_set_access_method(cur: Cursor):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, modified

@Leo-XM-Zeng Leo-XM-Zeng closed this Nov 6, 2024
@JelteF
Copy link
Collaborator

JelteF commented Nov 6, 2024

Did you close this on purpose? I was planning to merge this soon-ish.

@Leo-XM-Zeng
Copy link
Contributor Author

Did you close this on purpose? I was planning to merge this soon-ish.

No, no, I see a lot of changes, I plan to reorganize and resubmit.

@JelteF
Copy link
Collaborator

JelteF commented Nov 6, 2024

Ah alright. Sounds good, yeah we're refactoring a bunch of things to make the C vs C++ division clearer.

@Leo-XM-Zeng
Copy link
Contributor Author

Ah alright. Sounds good, yeah we're refactoring a bunch of things to make the C vs C++ division clearer.

This is good, I also want to try to do a little better, let you misunderstanding, sorry.

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.

2 participants