biggy icon indicating copy to clipboard operation
biggy copied to clipboard

Provide a way to get ROWID from an SQLite Table

Open sumo300 opened this issue 10 years ago • 7 comments

It'd be nice to have a way to get the ROWID of a SQLite table when no autoIncrementing key is provided in the model. This is an automatically created field for any SQLite table and is a 64-bit signed integer that is auto-incrementing from 1.

https://www.sqlite.org/autoinc.html

sumo300 avatar Apr 23 '15 11:04 sumo300

I think the original thinking here is that in order to behave predictably across platforms, Biggy wants you to explicitly specify a PK. Meaning (in the case of SQLite), either using PRIMARY KEY AUTOINCREMENT, or otherwise explicitly indicating a PK column (which does not have to be an auto-incrementing integer). While SQLite does provide ROWID as a handy default when the user doesn't explicitly add a PK, other relational platforms tend not to, and Biggy needs a PK of some sort to work properly.

I'm not opposed to investigating it though. Maybe set ROWID as the default for SQLite, or a useRowId option of some sort. I'm all about trying milk the in-built goodness specific to each supports platform.

I've been away from the code here for a bit, so I'll have to wrap my head around this again.

Meanwhile, feel free to shoot a PR if you'd like. If not, I'll still look into it. :-)

xivSolutions avatar Apr 23 '15 11:04 xivSolutions

I actually started looking at it, but didn't get very far yet. I think the issue I ran into was that if I specified a PrimaryKey explicitly, it was restricted to an Int32 or String and I needed an Int64 (ROWID being 64-bit). It's entirely possible that I'm doing something wrong, too!

Maybe important to note that I'm using the SqliteDocumentStore. Seems to create an id column that's hard-coded to INT.

I'm wondering if, for the SqliteDocumentStore, you'd be opposed to assume usage of the internal ROWID rather than explicitly creating id.

sumo300 avatar Apr 24 '15 01:04 sumo300

Reading some more SQLite docs, I think any column of type INTEGER that does not match in name to the built-in ROWID names (there's 3 of them), SQLite assumes it is simply an alias to ROWID if also defined as PRIMARY KEY. So, maybe it's as simple as a bug in the code assuming the value is int in C# when it's really long.

sumo300 avatar Apr 24 '15 01:04 sumo300

Yeah, I dug pretty ddep into the SQLite docs on this (for a different thing). I'm pretty sure there's a sensible way to leverage the ROWID column the way you mean. I just didn't want to set up a case where it looked like your could not explcitly set a PK in Biggy, as it would create inconsistency between supported platforms.

RE:

Maybe important to note that I'm using the SqliteDocumentStore. Seems to create an id column that's hard-coded to INT.

I'm wondering if, for the SqliteDocumentStore, you'd be opposed to assume usage of the internal ROWID rather than explicitly creating id.

I think there might be some traction here, or at least a way to serve both masters. Unless I've forgotten something, seems like the document store specifies the id behind the scenes anyway (although one might not always want it to be auto-incrementing). Hmmm...

It's late, and I work a long day tomorrow. Keep playing with it, and let's continue the conversation. Thanks for taking the time, I like where you're going, if it works, and doesn't back us into a corner somehow.

xivSolutions avatar Apr 24 '15 03:04 xivSolutions

I found the culprit. This code in DocumentStoreBase restricts to an Int32. I'm not sure yet if changing that to long has any side effects.

  // Key needs to be int, string:
  if (propertyType != typeof(int)
    && propertyType != typeof(string)) {
    throw new Exception("key must be either int or string");
  }

sumo300 avatar Apr 24 '15 14:04 sumo300

@xivSolutions Please check out the dc160e2 commit. I think I need more unit tests to ensure this change doesn't break anything. I want to make sure I'm on the right track.

sumo300 avatar Apr 28 '15 23:04 sumo300

Hey Michael -

Sorry, just got back from a business trip, catching up on email.

I'll try to checko this out by the weekend. Any progress since this email? I think if we can make the concept work, while preserving existing functionality you're on the right track.

On Tue, Apr 28, 2015 at 6:32 PM, Michael Sumerano [email protected] wrote:

@xivSolutions https://github.com/xivSolutions Please check out the dc160e2 https://github.com/xivSolutions/biggy/commit/dc160e28b4b2686d1752f0821ad0595919dadc7b commit. I think I need more unit tests to ensure this change doesn't break anything. I want to make sure I'm on the right track.

— Reply to this email directly or view it on GitHub https://github.com/xivSolutions/biggy/issues/27#issuecomment-97262761.

xivSolutions avatar Apr 30 '15 01:04 xivSolutions