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

Help required: query property #75

Closed
wants to merge 26 commits into from

Conversation

Buggaboo
Copy link
Contributor

@Buggaboo Buggaboo commented Nov 20, 2019

Remaining issues (2020 May 25)

  1. For example,.find(replaceNullWith:-2) on tLong works, but still not on the other integerish types.
  2. Doubles could be initialized to null (expected), but floats are set to 0.0f by default, thus making replaceWithNull unnecessary. Same thing as last year.
  3. Same syntax issue, we haven't addressed that at all. My gut feeling says we should apply extensions, and drop the generics.

So... I'm still stuck 6 months later. Ideas?

Remaining issues (2019 Nov 29)

  1. Signed negative values are returned as unsigned values by the find function on tShort and tInteger properties, e.g. find(replaceNullWith: -2)
  2. Float properties are never null, so regardless of the replaceNullWith parameter, uninitialized values will always return 0.0.
  3. Casting to PropertyQuery is an unnecessary pain.

Right now we have:

final query = box.query(tInteger.greaterThan(0)).build();
final qp = query.property(tInteger);

Where tInteger is referenced to return the concrete IntegerPropertyQuery, but the return type is still the limiting factor, i.e. the compiler will make it PropertyQuery again, because the generic param <PQ extends PropertyQuery> requirement applies.

We could change the syntax:

final q = box.query(tInteger.greaterThan(0)).build();
final qp = (tInteger.query(q)..distinct = true).average(); // Entity_.tInteger.query function is generated
q.close(); qp.close();

Copy link
Contributor

@vaind vaind left a comment

Choose a reason for hiding this comment

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

I think I've found the bug you were looking for WRT count() - should be fine after applying the proposed change (name of the function).

I'll still have to think about how to accomplish having a properly typed find() method directly on query().build().property() without having to cast.

lib/src/bindings/bindings.dart Outdated Show resolved Hide resolved
lib/src/bindings/signatures.dart Show resolved Hide resolved
lib/src/bindings/bindings.dart Outdated Show resolved Hide resolved
lib/src/bindings/signatures.dart Outdated Show resolved Hide resolved
test/query_property_test.dart Outdated Show resolved Hide resolved
Buggaboo and others added 10 commits November 29, 2019 10:33
Wrong ffi function referenced

Co-Authored-By: Ivan Dlugos <6349682+vaind@users.noreply.github.com>
Code analysis issue

Co-Authored-By: Ivan Dlugos <6349682+vaind@users.noreply.github.com>
Code analysis issue #2

Co-Authored-By: Ivan Dlugos <6349682+vaind@users.noreply.github.com>
Code analysis issue

Co-Authored-By: Ivan Dlugos <6349682+vaind@users.noreply.github.com>
Found 2 issues
* negative (signed) values for integers null replacements
* floats don't seem to become null at any point
@Buggaboo Buggaboo changed the title WIP - query property (with dart26 support) query property (with dart26 support) May 24, 2020
@Buggaboo Buggaboo changed the title query property (with dart26 support) Help required: query property (with dart26 support) May 24, 2020
@Buggaboo Buggaboo changed the title Help required: query property (with dart26 support) Help required: query property May 24, 2020
@greenrobot-team greenrobot-team changed the base branch from dev to main June 23, 2020 08:32
@greenrobot-team
Copy link
Member

@Buggaboo I'm trying to finish this in #108, feedback is still welcome.

@greenrobot-team
Copy link
Member

Merged as part of #108.

@Buggaboo Buggaboo deleted the query-property-dev-dart26 branch July 17, 2020 03:43
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