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

feat: Omit Columns from SQL statements (e.g uuid columns) #802

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Amnesthesia
Copy link

I've added two options for omitting columns from the SQL statements, to avoid having to modify class level self.ignored_columns since we've had issues with this affecting other code that runs at the same time. These options work as follows:

Omit the guid column from SQL statement, allowing it to generate the uuid and not be forced to NULL:
Model.import(values, omit_columns: [:guid])

Allow per-model decisions, e.g for recursive imports:
Model.import(values, omit_columns: -> (model, column_name) { [:guid] if model == Model })

Use a hash instead of a proc:
Model.import(values, omit_columns: { Model => [:guid] })

The second option is :omit_columns_with_default_functions boolean, to automatically find columns that have a default function declared in the schema, and omit them by default.

I've added two options for omitting columns from the SQL statements, to avoid having to modify class level `self.ignored_columns` since we've had issues with this affecting other code that runs at the same time. These options work as follows:

`Model.import(values, omit_columns: [:guid])` # Omit the guid column from SQL statement, allowing it to generate
`Model.import(values, omit_columns: -> (model, column_name) { [:guid] if model == Model })` Allow per-model decisions, e.g for recursive imports
`Model.import(values, omit_columns: { Model => [:guid] })` Use a hash instead of a proc

The second option is `:omit_columns_with_default_functions` boolean, to automatically find columns that have a default function declared in the schema, and omit them by default.

fix: Require AR 5.0+
@Amnesthesia
Copy link
Author

Can this be merged or is this already implemented upstream? We're currently pointing our Gemfile to this branch and can't update until uuid primary keys are supported

@jacob-carlborg-apoex
Copy link
Contributor

@Amnesthesia UUID primary keys already work, if they're following the standard naming convention, i.e. id. I'm using that on several tables in one of my applications. But for other columns it doesn't work.

@jacob-carlborg-apoex
Copy link
Contributor

@Amnesthesia BTW, it's already possible to specify which columns should be imported. Pass an array of symbols as the first argument to import. Second code example: https://github.com/zdennis/activerecord-import?tab=readme-ov-file#activerecord-models.

@Amnesthesia
Copy link
Author

Amnesthesia commented Jan 9, 2025

@jacob-carlborg-apoex This is for auto-generated UUID columns that are not primary keys :)

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