search

deakjahn / sqflite_web

A web plugin for https://github.com/tekartik/sqflite
Other
36 stars 20 forks source link

fixed the sqflite_web return format to match was was being returned in sqflite #3

Closed jamespet77 closed 4 years ago

jamespet77 commented 4 years ago

I can't be sure this is 100% correct. I am using jaguar orm and attempting to get this working with it. jaguar uses sqflite and works great with it. When I was pulling use the web version the formats were wrong and causing a lot of issues. I just converted it to match the same format that jaguar was expecting from sqflite. If this is wrong ignore it/.

deakjahn commented 4 years ago

@alextekartik Let's ping Alex to see what his opinion is.

robinbonnes commented 4 years ago

Edit: Sorry your correct @linuxjet. I can confirm this works. Can we merge this in?

deakjahn commented 4 years ago

If it works, we can for sure, the problem is that I had to fix an error yesterday and this made the two PR's conflict. Sorry... Could you update your side to be in sync once again?

alextekartik commented 4 years ago

For information, internally between the native and dart world in sqflite, query result data are coded as 'columns' and 'rows' for efficiency. The result is wrapped in a fake list of map (QueryResultSet):

https://github.com/tekartik/sqflite/blob/5cd8b1bc8e11dfe8b7485e20fb5c345e8431162f/sqflite_common/lib/src/collection_utils.dart#L21-L26

jamespet77 commented 4 years ago

I merged in the other changes and it is ready for merge

deakjahn commented 4 years ago

But I'll convert the map and list initializations to the more idiomatic way also suggested by pedantic. :-)

deakjahn commented 4 years ago

As I see, GitHub now screws up the permissions when something is merged. Well, I try a different route.

deakjahn commented 4 years ago

Well, I'll still struggle with this, but in the meantime, let's discuss it because, while it works and it returns a nice object, it seems to go against what Alex mentioned above. And chances are other parts of the Sembast packages might still expect that format.