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

Virtual column schema dump - default_function - Bring in recent postgressql adaptor change #383

Merged
merged 1 commit into from Oct 5, 2023

Conversation

wizardofelves
Copy link

@wizardofelves wizardofelves commented Sep 30, 2023

I switched from the official postgressql adaptor to this adaptor, and noticed that some virtual columns in my database started schema dumping incorrectly. In particular the as function would dump as nil

postgressql adaptor ->
  t.virtual "birthdate_text", type: :text, as: "birthdate", stored: true
postgis adaptor ->
  t.virtual "birthdate_text", type: :text, as: nil, stored: true

I did some investigation and it looks like your adaptor is missing a recent change @fatkodima made that fixes this issue. here

rails/rails@7fa2720

I copied his change and added a spec to cover the scenario. I tried to do my best to adhere to the style and format of the existing tests, but can make additional changes if needed.

Thank you for your work on this project.

@wizardofelves wizardofelves changed the title fix for virtual column definition - bring in https://github.com/rails… Virtual column definition - default_funtion - Bring in recent postgressql adaptor change Sep 30, 2023
@wizardofelves wizardofelves changed the title Virtual column definition - default_funtion - Bring in recent postgressql adaptor change Virtual column schema dump - default_function - Bring in recent postgressql adaptor change Sep 30, 2023
@wizardofelves wizardofelves reopened this Sep 30, 2023
Copy link
Member

@BuonOmo BuonOmo left a comment

Choose a reason for hiding this comment

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

Thanks for contributing.

LGTM! @keithdoggett I'll let you give a second opinion.

@keithdoggett keithdoggett merged commit 1b7588b into rgeo:master Oct 5, 2023
20 checks passed
@keithdoggett
Copy link
Member

Thanks @wizardofelves!

@keithdoggett keithdoggett mentioned this pull request Oct 5, 2023
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.

None yet

3 participants