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

AUTO_INCREMENT and LocalStorage #462

Closed
error39 opened this issue Nov 19, 2015 · 25 comments
Closed

AUTO_INCREMENT and LocalStorage #462

error39 opened this issue Nov 19, 2015 · 25 comments

Comments

@error39
Copy link

error39 commented Nov 19, 2015

When using localstorage, AUTO_INCREMENT doesn't seem to function, as though without specifying a storage method it does work, is this intended behaviour or am I implementing it incorrect?

my implementation of A_I with localstorage: http://jsfiddle.net/gym7L42r/

@mathiasrw
Copy link
Member

Nice catch - a clear bug...

Made a version making it easy to compare expected and actual output

http://jsfiddle.net/hfcpcnxt/

@error39
Copy link
Author

error39 commented Nov 20, 2015

okay, glad I did report it then, will improve alasql a tiny bit then :)
Really appreciate all the work that has gone into this GREAT project, thumbs up!

@mathiasrw
Copy link
Member

👍

@glaucocustodio
Copy link

Same problem here, it does not generate id...

@error39
Copy link
Author

error39 commented Nov 28, 2015

I was wondering, will this be fixed?

@mathiasrw
Copy link
Member

Sorry my short answer from a phone. Yes it will be solved - it's an error - but can't tell you when. In this moment we are transforming the code base to ES6. It depends on when people like @agershun take the time to get into the nitty gritty...

@error39
Copy link
Author

error39 commented Dec 23, 2015

Okay, I understand the problem.
But do you have a slight idea when this (kind of basic) functionality will be added/fixed then?
In - like - a month or more likely 6 months or a year?
You've all done a great job with this library and for future projects I'd be happy to know if it's actively maintained, or if it's just a more or less finished project?

Wish I could be of some help on the subject, but my Javascript skills are far from suited for these kind of projects.

Good luck with the codebase rewrite, a huge job, I guess..

@mathiasrw
Copy link
Member

Sorry i cant come with a better answer... I really have no idea when someone will feel in the mood to dig into the issue.

From November 27 to December 27 we had the following activity:

  • 6 authors have pushed 44 commits (4 Pull requests)
  • 86 files have changed
  • 19 Issues created
  • 10 Issues closed

So guess some people still seek to improve the library.

I am sure that if you have an urgent need you can hire @agershun to spend the time needed to fix this.

@error39
Copy link
Author

error39 commented Dec 27, 2015

Thanks, I'll wait till someone will pick up the issue.
In the mean time I'll stick to a custom workaround which auto-increments at every INSERT statement

@agershun
Copy link
Member

Sorry for late answer, I will try to fix it in one or two days.

Отправлено с iPhone

27 дек. 2015 г., в 11:06, error39 notifications@github.com написал(а):

Thanks, I'll wait till someone will pick up the issue.
In the mean time I'll stick to a custom workaround which auto-increments at every INSERT statement


Reply to this email directly or view it on GitHub.

@error39
Copy link
Author

error39 commented Dec 28, 2015

Thanks a lot @agershun!

@agershun
Copy link
Member

It was harder than expected... Sorry for delay

@seb-ster
Copy link
Contributor

Is there any progress on this issue?
It's not working in on W10, FF45 with AlaSQL v0.2.5 and local storage.
[edit]
checked out the local storage property in the DOM. The table definition seems correct:

{"columnid":"_id","dbtypeid":"INTEGER","identity":{"value":1,"step":1}}

@ellisgl
Copy link

ellisgl commented May 5, 2016

Same thing for FileStorage.

@mathiasrw
Copy link
Member

#722 is also having the problem.

Any help on this one would be aprechiated.

@seb-ster
Copy link
Contributor

seb-ster commented Sep 20, 2016

@mathiasrw So, i tracked it down, at least for Local Storage:

Normal ('memory') storage, uses the table.insert() method.
This method has a for loop which checks if an IDENTITY (Autoincrement) is defined for any of the columns that are being inserted. If this is so, it sets the column value accordingly.

Local Storage uses the LS.intoTable() method.
This method has no such for loop.
I got it to work by inserting a slightly modified version of the table.insert() for loop, just after the table is selected with LS.restoreTable().

for(var columnid in tb.identities){
var ident = tb.identities[columnid];
value[0][columnid] = ident.value++;
}

The main difference is the need for an increment operator on ident.value

file: alasql.js
version: 0.3.2
system: FF 48.0.2 on W10

@mathiasrw
Copy link
Member

mathiasrw commented Sep 21, 2016

@seb-ster

Really really awesome - thanks :)

Would you like to make a pull request so the code can correctly be attributed to you?

If not, let me know and Ill implement your suggestion...

@seb-ster
Copy link
Contributor

seb-ster commented Sep 21, 2016

@mathiasrw
My solution is still a little hackish. Especially the increment operator should atleast be replaced by the identity step value.
I did it this way because the table (and identity) is loaded via LS.restoreTable() , I didn't have direct access to the identity value as in the table.insert() for-loop.

The table.insert() method, has a second for-loop that raises the identity.value by an amount of identity.step after the row is inserted.

for(var columnid in table.identities){
var ident = table.identities[columnid];
ident.value += ident.step;
}

This then also needs to be implemented in LS.intoTable() after the row is concated to the database.

Also, this solution probably also needs implementing in other DB engines.

Would you review and implement the solution for me?
Thanks!

@mathiasrw
Copy link
Member

Sure!

@kanghj
Copy link
Contributor

kanghj commented Feb 6, 2017

It looks sufficient to increment ident.value in LS.restoreTable. We can obtain the value of the step using ident.step, so the solution should look something like:

for (var columnid in tb.identities) {
	  var ident = tb.identities[columnid];

	  for (var index in value) {
	    value[index][columnid] = ident.value;
	    ident.value += ident.step;
	  }      
}

?

@alshdavid
Copy link

Still an issue for me. Chrome & local storage

@calebeaires
Copy link

Version alasql 0.4.8 still have this issue. Auto increment does not work. My project is using FILESTORAGE DATABASE

@classofoutcasts
Copy link

Solved? Great! I left Alasql a while ago as it didn't seem to be actively maintained any more, might reconsider that now though.

@calebeaires
Copy link

@mathiasrw Very thanks to that.

Our team were considering use alasql as some of the repositories of a PowerBI alternative. We stopped becouse of this index issue and another one: we tried to do a left join, but it dident bring results using FILESTORAGE DATABASE

We will keep our eyes on it, it is a amzing project.

@mathiasrw
Copy link
Member

Its driven by people contributing - if you need the features fixed it would help greatly if you look into where the problem is located. Like #1013 - so that we together can improve the quality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants