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

adds support for inserting null values with columns #174

Merged
merged 4 commits into from
Jan 14, 2019

Conversation

jaredanderton
Copy link
Contributor

Currently, to insert a null value (from a nil/optional value), you must do an insert where you specify every column and a value for every column in the insert (so you cannot do a partial insert). This creates overhead each time a migration alters the table structure.

This change introduces support for a partial insert with optional values.

@CLAassistant
Copy link

CLAassistant commented Jan 9, 2019

CLA assistant check
All committers have signed the CLA.

@kilnerm
Copy link
Contributor

kilnerm commented Jan 11, 2019

@jaredanderton, Thanks for your submission. If we take this PR as it currently stands we are going to need to do a new major release of SwiftKuery as the update to the enum is potentially breaking. I think this highlights a big issue with the enum as a new major is not something that we really want to do so soon after SwiftKuery 3.0.

Would you be willing to refactor your PR and simply hardcode the “NULL” into the default case for packType rather than using the queryBuilder approach? We can then get the fix released and I will work on removing the enum restrictions under issue #175 ready for our next major version release.

@jaredanderton
Copy link
Contributor Author

@kilnerm Of course. Thank you for the input. PR has been refactored.

@kilnerm kilnerm self-assigned this Jan 14, 2019
@kilnerm kilnerm self-requested a review January 14, 2019 11:26
Copy link
Contributor

@kilnerm kilnerm left a comment

Choose a reason for hiding this comment

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

This is a good addition. Thanks for the rework!

@kilnerm kilnerm merged commit f2f78f2 into Kitura:master Jan 14, 2019
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.

3 participants