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

Refactor generated to returning keyword in order to return the correct type #444

Merged
merged 1 commit into from
Jul 24, 2016

Conversation

gustavoamigo
Copy link
Contributor

@gustavoamigo gustavoamigo commented Jul 11, 2016

Fixes #427

Problem

Right now, the generated parameter in schema is limited to Long. If the primary key is not a type that can be casted safely to Long, like UUID, it limits its use.

Solution

Create new keyword returning which will allow other types to be returned. Example:

case class Entity(name: String, id: Option[UUID] = None)
val q = quote {
        query[Entity].insert.returning(_.id)
      }
val id: Option[UUID] = db.run(q)(Entity("test"))

Notes

Additional notes.

Checklist

  • Unit test all changes
  • Update README.md if applicable
  • Add [WIP] to the pull request title if it's work in progress
  • Squash commits that aren't meaningful changes
  • Run sbt scalariformFormat test:scalariformFormat to make sure that the source files are formatted

@getquill/maintainers

@mdedetrich
Copy link
Collaborator

@gustavoamigo Looking good from here!

def delete: Delete[T, Long]
}

implicit class InsertUnassignedAction[T](i: T => UnassignedAction[T, _] with Insert[T, _]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we avoid the implicit class here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll try

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no lucky trying to remove the implicit class. I tried to create a trait that extends T => UnassignedAction[T] with Insert[T], but it breaks macros. It seems to be easier to just use implicit class.

@gustavoamigo
Copy link
Contributor Author

@getquill/maintainers the ActionApply will not work inherited from Function1 and both type parameters open. I will have to refactor it to use a generic ActionApply that does not inherit Function1. In this branch ActionApply is disable for the quill-jdbc and quill-async module. Are you guys ok if I fix this issue in another PR, so I can finish this one earlier?

BTW, there are a few things I need to do like docs and tests, but this branch is ready for testing.

@gustavoamigo gustavoamigo changed the title [wip] Refactor generated to returning keyword in order to return the correct type Refactor generated to returning keyword in order to return the correct type Jul 21, 2016
@gustavoamigo
Copy link
Contributor Author

@getquill/maintainers This PR is ready for review (if it builds). As I said, I removed ActionApply from quill-jdbc and quill-async, since it is buggy to inherit from Fuction1. I would like to refactor ActionApply in another PR.

@@ -821,8 +820,8 @@ The `forUpdate` quotation can be reused for multiple queries.
The same approach can be used for `RETURNING ID`:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we could remove this example or replace it by another one now that returning id is supported by the DSL.

@gustavoamigo
Copy link
Contributor Author

@getquill/maintainers all right, I think this PR is ready for merging.

@fwbrasil fwbrasil merged commit f9bd119 into master Jul 24, 2016
@fwbrasil fwbrasil deleted the returning branch July 24, 2016 23:53
jilen pushed a commit that referenced this pull request Jun 11, 2024
Bumps [actions/checkout](https://github.com/actions/checkout) from 4.1.3 to 4.1.5.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v4.1.3...v4.1.5)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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