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

Query can't use method parameter more than once #142

Closed
rosstimothy opened this issue May 20, 2019 · 12 comments · Fixed by #509
Closed

Query can't use method parameter more than once #142

rosstimothy opened this issue May 20, 2019 · 12 comments · Fixed by #509
Labels
feature Request and implementation of a feature (release drafter)
Milestone

Comments

@rosstimothy
Copy link

Title says it all..

@Query('SELECT * FROM my_table WHERE column_a like :my_param or column_b like :my_param')

results in

SQL query arguments and method parameters have to match.

Looks like the generator doesn't take into account a parameter of the same name being used more than once in the query.

@vitusortner
Copy link
Collaborator

I've decided some time ago to make use of sqflite's parameter binding capabilities. A clear downside is the behavior you mentioned. At that time I wasn't able to come up with a solution that solves this issue while still using parameter binding. I'll revisit the code and will report back here.

@vitusortner vitusortner added the feature Request and implementation of a feature (release drafter) label May 21, 2019
@vitusortner vitusortner added this to the Version 1.0 milestone Aug 27, 2019
@VadimOsovsky
Copy link

@vitusortner the fix would be highly appreciated! :)

@mqus
Copy link
Collaborator

mqus commented May 16, 2020

Small hint for a workaround for now (I'm trying to do a better quickfix): use ?1 instead of :my_param and it should work the way you expect it to. Use ?n to use the nth parameter. This only works for normal parameters, not for lists in IN clauses.

Sqlite can already map single parameters to multiple locations and also supports named parameters (including reordering and multiple-use). Sadly sqlite on android only supports positional parameters, so sqFlite also can't support it easily (as explained in the linked issue).

EDIT: ?n works for Strings, not integers, until sqflite has adapted. Another android limitation :/

@mqus mqus mentioned this issue Jun 15, 2020
23 tasks
@mqus mqus linked a pull request Jun 15, 2020 that will close this issue
23 tasks
@cassioseffrin
Copy link
Contributor

@mqus your quickfix will be very very useful. Do you could give us an estimative when it will be released to production?

@mqus
Copy link
Collaborator

mqus commented Aug 12, 2020

This quickfix is(should be) working right now. But you will have to change it back after #361 is merged, which will solve this issue.

@cassioseffrin
Copy link
Contributor

Any estimative how long this merge will take, also when this will be released in production? There are 39 commits in there. If there is a separated branch may I could help to test it.

@mqus
Copy link
Collaborator

mqus commented Aug 19, 2020

#361 is a separate branch. You can only open a PR if you have a separate branch.
grafik

PS: Regarding the estimate: Pretty much everything that happens and is talked about is documented in this repository publically. So you can see if there is anything happening that is affecting my original estimate. Right now, the answer is still: Depends on the review and on some tests that are still missing, but I'm hopeful that this happens this year and if we're fast in a month (including a release). If you want to get involved and help, just add a comment or a review in #361. If you want to know what is happening in #361, enable notifications for this PR.

@cassioseffrin
Copy link
Contributor

I have tried to add integrate-sqlparser branch in pubspec.yaml:

dependencies:
  flutter:
    sdk: flutter

  floor:
    git:
      url: git://github.com/mqus/floor.git
      path: floor
      ref: integrate-sqlparser

However I am getting an error:
Error on line 17, column 11: Invalid description in the "floor" pubspec on the "floor_annotation" dependency: "../floor_annotation/" is a relative path, but this isn't a local pubspec.

Seems like the path is invalid, but I can't get it to work even without path.

@mqus
Copy link
Collaborator

mqus commented Aug 19, 2020

I don't like to repeat myself, but if you have issues with #361, post them there! This issue here is not the place for it.
And also: if you would have searched a bit, you would have discovered #288 or something similar in the internet, which tells you that you can't refer to a dependency via git, if it has dependencies which have relative paths themselves (like all development branches in floor). You will have to clone it locally and use something like:

dependencies:
  floor:
    path: floor_repo/floor

If you want to enter it into git, you can use git submodule. This is not perfect but it somewhat works.

@cassioseffrin
Copy link
Contributor

@mqus. I think you are terrible wrong:

Look how it's work, even in floor repo there are [people referring to a git repo directly] (#292 (comment))

And plus, there are some nice packages that works with github and pub.dev. If your repo doesn't work it don't mean this doesn't work. it's very useful. It's not only work with flutter but also with react-native and many other technologies. I recommend you to don't take a stackoverflow answer as the truth.

If you still in doubt check installation guide of qr.flutter:
https://github.com/lukef/qr.flutter

And finally check flutter oficial docs about how to work with git dependency
dependencies:
plugin1:
git:
url: git://github.com/flutter/plugin1.git

https://flutter.dev/docs/development/packages-and-plugins/using-packages

@mqus
Copy link
Collaborator

mqus commented Aug 20, 2020

To repeat:
The dependency chain your_package ---git-dependency-with-path---> floor ---path-dependency---> floor_annotation does not work! Which is exactly what the linked stackoverflow page and the linked issue are about. This is not about using a git dependency and a relative path at the same time for the same dependency, but about the chain in this particular order, which does work in other programming languages but not in dart/pub. If you don't believe stackoverflow, believe the dart devs: dart-lang/pub#449 (EDIT or dart-lang/pub#2447 which fits better but is a duplicate).

Sure you can reference any dependency via git. But only if they themselves don't have path-relative dependencies (like floor has for floor_annotation). The comment you linked at forked floor and changed the pubspec, see https://github.com/tulioccalazans/floor/blob/develop/floor/pubspec.yaml#L16

Of course it works in other repositories because they either don't have inner dependencies within the same repository that are so tightly coupled that they have to be developed together or they made different decisions. The issue I referenced had a PR linked where the options were explored, #298. To summarize the option of using git references for the inner dependencies: if you edit floor and have to change more than one inner package or something in the middle, you will have pain. So the current way is to have relative locations and replace them with versioned release numbers once we create a release.

This minimizes floor dev pain at the cost of making snapshot dependencies(which we experience as rarely used) more difficult to manage.

@cassioseffrin
Copy link
Contributor

Ok @mqus

I have this doubt mainly because your repository https://github.com/mqus/floor/tree/integrate-sqlparser and tulioccalazans repository https://github.com/tulioccalazans/floor seems to have the same directory structure and I was able to use his repository directly in pubspec.yaml but not yours.

This dependency chains is very difficult to understand in first view, also it's sluggish to download the files and put them to project tree manually. For project collaborations it's not practical. But ok, some things aren't easy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Request and implementation of a feature (release drafter)
Development

Successfully merging a pull request may close this issue.

5 participants