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 virtual columns #163

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

robbevp
Copy link

@robbevp robbevp commented Nov 14, 2024

Hi there 👋

Thanks for this active fork! I had a personal fork with some changes for the longest time.

One of these changes was support for virtual/generated columns.
At the moment, the following table would create a slightly confusing annotation

create_table :users do |t|
  t.string :name
  t.virtual :name_upcased, type: :string, as: "upper(name)", stored: true
end
# Table name: users
#
#  name                :string
#  name_upcased :string
class User < ApplicationRecord

With these changes, this would generate the following:

# Table name: users
#
#  name                :string
#  name_upcased :virtual(string)
class User < ApplicationRecord

I wasn't a 100% sure where you'd want these changes exactly. I'm happy to make further changes, or add some more tests.

One additional change could be to also place the function in the annotation, but this would maybe better in a further PR. I've tried this in the past and noticed that it can become unwieldy once the function becomes more complicated

# Table name: users
#
#  name.                :string
#  name_upcased :virtual(string)    upcase(name)
class User < ApplicationRecord

@drwl
Copy link
Owner

drwl commented Nov 15, 2024

Hi @robbevp thanks for putting up a PR. I don't have any objections here, but let me play around with this.

At a high level, my only concern is that adding this without an option flag could affect others.

@drwl
Copy link
Owner

drwl commented Nov 16, 2024

I had a chance to play around with this locally. The direction seems fine. I'm curious what it looks like with the functions as well.

I'd be okay taking the current changes, but would prefer there be a corresponding option flag, perhaps something like show_virtual_columns? I'm open to a different/better naming.

@robbevp
Copy link
Author

robbevp commented Nov 19, 2024

Hi @drwl
I added the option - I couldn't think of anything better, so went with your suggestion.
Since we now have an option, I also added a separate commit that will print the function, so you can try this as well. I'm ok with dropping that commit if you don't want those changes (yet).

@robbevp robbevp force-pushed the enhc/support-virtual-columns branch from 7d4ba6f to c4ef34f Compare November 20, 2024 06:58
@robbevp
Copy link
Author

robbevp commented Nov 20, 2024

Just saw that I forgot to lint my files yesterday, this is fixed now

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.

2 participants