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

Expected StartObject token for type Nullable #63

Closed
c-emin opened this issue Mar 10, 2025 · 8 comments
Closed

Expected StartObject token for type Nullable #63

c-emin opened this issue Mar 10, 2025 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@c-emin
Copy link

c-emin commented Mar 10, 2025

I am getting this error now with the latest Nuget version 1.0.11
Unhandled exception rendering component: Expected StartObject token for type Nullable`1.

If I use nullable reference in the Model:
public DateTime? modifiedOn { get; set; }

@magiccodingman magiccodingman self-assigned this Mar 10, 2025
@magiccodingman
Copy link
Owner

Thank you! I will validate what's going on and repair asap. Thanks for the report!

@magiccodingman
Copy link
Owner

I've repeated the Expected StartObject token for type Nullable. error. Issue is the new serializer is targeting types and needs to recognize if things are a simple of complex type. This was a simple, but was missed because it wasn't looking at the underlying type like it should have been checking.

I've resolved the issue and am now validating in testing it's all clear. Because one of my tests invoked:

x.DateOfBirth.Value.Year < 1980

Which caused the error "Unsupported binary expression. Expression" which just means it's unsupported. But I'm going to see if I can make Value based binary operations properly recognized and working full nullable types all around moving forward alongside this patch. Fingers crossed it's not too much work.

Will have a patch out likely in the next few hours if not sooner (hopefully) with the patch. Expect 1.0.12 after I confirm my tests and see if I can get nullable binary expressions to work.

@magiccodingman magiccodingman added the bug Something isn't working label Mar 10, 2025
@magiccodingman
Copy link
Owner

magiccodingman commented Mar 10, 2025

Okay the bug is fixed, but I'm going to make that date time binary operator supported like I want in a future update because that'd be really nice. But I've added to my To Do list to support:

x.DateOfBirth.Value.Year < 1980
x.DateOfBirth.GetValueOrDefault().Year < 1980

After reviewing the code, it's likely not too crazy hard, but it's not a quick addition either. I also noticed some other issues too. Plus the ability to handle date times like that is nice, but the .Value and/or GetValueOrDefault() would be nice additions as well to support for nullable values in the future.

Unit Tests

I tested the following:

WhereExample = (await manager.Where<Person>(x => x.DateOfBirth.Equals(null)
).ToListAsync());

WhereExample = (await manager.Where<Person>(x => !x.DateOfBirth.Equals(null)
).ToListAsync());

WhereExample = (await manager.Where<Person>(x => x.DateOfBirth == null
).ToListAsync());

WhereExample = (await manager.Where<Person>(x => x.DateOfBirth != null
).ToListAsync());

WhereExample = (await manager.Where<Person>(x => x.Name.StartsWith("c", StringComparison.OrdinalIgnoreCase)
|| x.Name.StartsWith("l", StringComparison.OrdinalIgnoreCase)
|| x.Name.StartsWith("j", StringComparison.OrdinalIgnoreCase) && x._Age > 35
|| x.Name.Contains("bo", StringComparison.OrdinalIgnoreCase)
|| x.DateOfBirth == null
).OrderBy(x => x._Id).Skip(1).ToListAsync());

I tested a few others as well successfully. The bug should be resolved.

Additional Bug fixes

I'll detail this in the next release, but in the patch will be some additional nice fixes:
1.) Can now compare to null. So, (x.DateOfBirth == null) operations are now supported.
2.) Resolved a Skip bug that could occur in complex LINQ statements.
3.) Fixed null serialization scenarios.
4.) Repaired a couple potential != secnarios

Conclusion:

The changes are currently in main. I will be releasing 1.0.12 soon. Thanks for the bug report, it's appreciated as the past couple updates have been very large refactors adding deep levels of functionality. So I appreciate the patience.

@c-emin
Copy link
Author

c-emin commented Mar 11, 2025

That fixed the error. I am able to run my app now.!!
Thanks for the super quick update!

I did a little benchmark of one of my processes. (Writing to IndexedDb, Reading from IndexedDb)
before v1.0.11: 32-34 secs
v1.0.12: 42-44 secs

Not sure what might be causing the slower performance. ??

FYI, I am getting a lot of console messages in my "Release" deployed app "Received streamRef"

@magiccodingman
Copy link
Owner

magiccodingman commented Mar 11, 2025

Oh thank you @c-emin ! Also those console logs are supposed to show up only in DEBUG. But I guess something is wrong, I fixed that in main and will make another Nuget update with that fix.

Performance Issue

As for the longer performance time. That's for sure odd. Though, I have some suspicions. Firstly, was the 30% longer time test after the first run (aka caching was initiated) or was it from the first run you made?

Additionally, I didn't think it could be my changes, but I had that, "ohhhh" moment as I was about to pat myself on the back lol. I don't think there's any remote chance it's the C# code changes. But there could be really only 1 change I can think of.

Theory

Within the universal translation layer in JS where I convert your LINQ to the indexDB equivalent, the combineQueries was changed like so:

async function combineQueries() {
    for (const conditions of orConditionsArray) {
        await processIndexedQuery(conditions[0]);
    }
    results = applyArrayQueryAdditions(results, QueryAdditions);
    if (uniqueResults) {
        const uniqueObjects = new Set(results.map(obj => JSON.stringify(obj)));
        results = Array.from(uniqueObjects).map(str => JSON.parse(str));
    }
    return results;
}

Which I removed the original asynchronous call which used to call things in parallel. Which was additionally done purposely because that parallel call was in fact faster as it ran each query separately, but I also recognized that the parallel call was causing issues since order does matter!

Theory Meaning

The code in v1.0.11 and prior did not run queries correctly because they were not ordered right nor run synchronously like they should have. Which the v1.0.12 fixed. So this may be the result of the query working as it's supposed too. And that would mean version 1.0.11 is not a good test because it was inherently broken.

Final thoughts

There was a great deal of fixes in general in 1.0.11 and I think that the 1.0.12 patch fixed even more issues that caused the query to run correctly, but in return now shows the real results as it always should have. Though just so you know I took this very seriously and I think this is likely the application running as it's supposed too versus the original broken code. But if you think otherwise or disagree, please let me know so I can better investigate.

New Documentation

But, remember that the LINQ to IndexDB translation is trying its best to convert your relational like desired LINQ response into a non relational database that does not have the same capabilities. It's a very interesting process actually and this inspired me to document this properly finally here:
Magic IndexDB - How LINQ to IndexedDB Works

That may potentially explain some slow downs.

This is NOT to say that the code cannot see any improvements! There's many improvements to be made and that I'm working on. Especially by translating a lot of the old logic that processed information in memory to the newly implemented cursor feature. Which allows us to emulate significantly more abilities within the query.

Conclusion

I will continue to investigate this, but I do believe this performance issue is due to the code working correctly. And that previous benchmarks aren't fair to compare. But if you have any benchmarks of this process prior to 1.0.11 and many of the large refactors, I'd love to see it! And is it a very hardcore query? Or large data pulls? I'm just curious, because that's one hell of a long query! And I wonder if I can build this to be better 🥇

But any information that you provide could help me build better tests. Plus help optimize, make faster, and more reliable code in the future! So any ideas, or examples of what you're doing, it would all really help!

@c-emin
Copy link
Author

c-emin commented Mar 11, 2025

Thank you for that detailed response! I mainly wanted to just give you feedback, not complaining.😊

In this particular process, I am looping through and downloading 57 Blob images, decrypting them, and storing them using AddAsync into the Indexed DB. Then I perform a GetAllAsync to load the images into the ImageModel.

I'm not exactly sure what the prior version would be (perhaps 1.0.10?), I used the github project files that I downloaded on 2/13.
(This was right after @yueyinqiu completed the work on OpenAsync to be able to store custom named multiple DB's.)
BTW. This works perfectly!!

Which this is the reason I would like to only query certain columns, such as Id, imageName, etc... without pulling all the binary image data, just to see if I have that info in the browser IndexedDB cache.

Right now I solved it by creating a separate "Shadow" table without the image data, to be able to read the meta data quicker.

@magiccodingman
Copy link
Owner

@c-emin That's great! Just a heads-up—significant changes were made to querying starting in v1.0.11. More details can be found here:
📖 Magic IndexDB - How LINQ to IndexedDB Works
📖 v1.0.11 - Update Notes

These updates enable column-based queries for both indexed and non-indexed fields. Thanks to improved logic, the system now uses cursors for in-query searching, ensuring optimal memory use. There's a lot of planned updates to utilize cursors even further to reduce in memory use significantly more!

Key Improvements Since v1.0.11:
No more uncertainty—if something is queryable, it should work as expected.
No more full-memory loads (unless explicitly intended).
Major performance improvements in querying and data retrieval.

🔹 Best Practice: Instead of GetAll(), I recommend targeted queries to avoid unnecessary memory loads.

🚀 Feedback Welcome!

If you notice anything out of the norm—like unexpected full-memory loads—please let me know! That would be a bug I'd want to resolve.

Also, heads-up! I started a discussion on a major refactor that will impact syntax in future versions:
📌 Issue #60 – Planned Refactor

The goal? Even faster, even easier, even better. Keep an eye on that thread for updates!

Hope this helps with your project, and I appreciate your feedback! 😊

@magiccodingman
Copy link
Owner

magiccodingman commented Mar 11, 2025

@c-emin Just a heads up, I've found additional odd things going on in this new update. Stay tuned for updates as there may be more performance improvements that may make a difference ;) The thing I was talking about before is 100% the initial performance discovery. But there's a lot of performance tuning coming up in a really major refactor that'll shift this project to a very akin version of LINQ to IndexDB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants