memtrip / SQLKing

SQLKing is an Android SQLite ORM powered by an annotation preprocessor, tables are defined by Table annotations and CRUD classes expose an expressive api for executing SQLite queries. @memtrip
Other
21 stars 9 forks source link

FYI - contributions - Composite Foreign Keys, Composite indexes #18

Open Trellian opened 8 years ago

Trellian commented 8 years ago

Hi Sam,

Just to let you know, I'm busy adding support for composite Foreign Keys and indexes, will issue a pull request shortly :)

Cheers, Adrian

samkirton commented 8 years ago

Hi Adrian,

That is great to hear! Could you rebase your changes with the latest master? Last week I added changes for Primary Keys.

Cheers, Sam

Trellian commented 8 years ago

Sam,

Yes, I have already pulled in the Primary Key changes. You pipped me to the post there, I had done exactly the same as you for the primary keys.

The way I have structured it, @column no longer has the index= option, they are table based now. Are you ok with that?

Regards, Adrian

On 17 Sep 2016 09:13, Samuel Kirton wrote:

Hi Adrian,

That is great to hear! Could you rebase your changes with the latest master? Last week I added changes for Primary Keys.

Cheers, Sam

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/memtrip/SQLKing/issues/18#issuecomment-247754490, or mute the thread https://github.com/notifications/unsubscribe-auth/ABCOrAjuu9QXe9sfCSnfmjQHewWPM_RIks5qq5L9gaJpZM4J_hpM.

samkirton commented 8 years ago

hmmm I think I preferred index in @Column, I assume if you have it in @Table you must reference the column by a name string? This has more room for a typo. What do you think?

Sam

Trellian commented 8 years ago

I agree, but how else to build composite indexes?

Adrian

On 17 Sep 2016 09:18, Samuel Kirton wrote:

hmmm I think I preferred index in @Column https://github.com/Column, I assume if you have it in @Table https://github.com/Table you must reference the column by a name string? This has more room for a typo. What do you think?

Sam

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/memtrip/SQLKing/issues/18#issuecomment-247754713, or mute the thread https://github.com/notifications/unsubscribe-auth/ABCOrE15XLjphBHYRUvHIM2DJKt4ZLclks5qq5RbgaJpZM4J_hpM.

Trellian commented 8 years ago

I guess it's about the same amount of room for typos as defining indexes in SQL DDL?

Adrian

On 17 Sep 2016 09:18, Samuel Kirton wrote:

hmmm I think I preferred index in @Column https://github.com/Column, I assume if you have it in @Table https://github.com/Table you must reference the column by a name string? This has more room for a typo. What do you think?

Sam

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/memtrip/SQLKing/issues/18#issuecomment-247754713, or mute the thread https://github.com/notifications/unsubscribe-auth/ABCOrE15XLjphBHYRUvHIM2DJKt4ZLclks5qq5RbgaJpZM4J_hpM.

samkirton commented 8 years ago

Would it be possible to have the functionality in both? Basic users can stick to @Column and more advanced users can define their composite as part of @Table?

Trellian commented 8 years ago

Yes. I will make it so ;)

Adrian

On 17 Sep 2016 09:21, Samuel Kirton wrote:

Would it be possible to have the functionality in both? Basic users can stick to @Column https://github.com/Column and more advanced users can define their composite as part of @Table https://github.com/Table?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/memtrip/SQLKing/issues/18#issuecomment-247754816, or mute the thread https://github.com/notifications/unsubscribe-auth/ABCOrE2cV_iCEfg9Ex6AL6utgNW6Zg5gks5qq5TogaJpZM4J_hpM.

samkirton commented 8 years ago

ok great, also it would be great if you could write some integration tests in the client project that satisfy the implementation :)

Trellian commented 8 years ago

I'll give it a shot... I'm new to the integration test scene, and also new to contributing via Git, so there might be a few screw-ups along the way that I might need help with. Some git concepts still confuse the heck out of me.

I've obviously been following your code for the testing, will atempt to do it the same way. I'm a little short on time though, since this is a production project, but I'm really enjoying contributing.

I wrote many, many DB templates in Clarion's template language, and I have a bushel full of ideas and additions for SQLking.

/Adrian

On 17 Sep 2016 09:23, Samuel Kirton wrote:

ok great, also it would be great if you could write some integration tests in the client project that satisfy the implementation :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/memtrip/SQLKing/issues/18#issuecomment-247754902, or mute the thread https://github.com/notifications/unsubscribe-auth/ABCOrOIJauSfcB-H97KM0_QpSMacaIFIks5qq5VvgaJpZM4J_hpM.

samkirton commented 8 years ago

If you are going to use the new version in production its especially important that we get the integration test coverage, this is something I can write if you are short on time. Have you been running the existing test suite while you have been developing? This will catch any regression issues early on.

Cheers, Sam

Trellian commented 8 years ago

Yes, I agree. I've been running the existing tests, and have also corrected a bug in CREATE TABLE that was leaving off the last character before the closing ');' in some cases (possibly caused by me)

/Adrian

On 17 Sep 2016 09:32, Samuel Kirton wrote:

If you are going to use the new version in production its especially important that we get the integration test coverage, this is something I can write if you are short on time. Have you been running the existing test suite while you have been developing? This will catch any regression issues early on.

Cheers, Sam

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/memtrip/SQLKing/issues/18#issuecomment-247755270, or mute the thread https://github.com/notifications/unsubscribe-auth/ABCOrNu2cgBVmPCmfa2C-gwWtIRXC-JJks5qq5emgaJpZM4J_hpM.

samkirton commented 8 years ago

ok great!