[Cayenne-dev] [jira] Updated: (CAY-1116) [patch] add generated PK support to SQLite

[ https://issues.apache.org/cayenne/browse/CAY-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13143#action_13143 ]

Andrus Adamchik commented on CAY-1116:
--------------------------------------

I am deferring to Tore to judge the patch details, as it is related to the merge function that he knows better than anyone. So let me just make a few general observations.

1. Is that really a "add generated PK support to SQLite" patch? It changes DbAdapter API, so I'd like to have reasons for this change stated a bit more explicitly in Jira, as DbAdapter is one of the important integration points in Cayenne, and I'd rather we keep it clean and change only if absolutley needed.

2. I am a bit concerned about DbAdapter methods getting too specialized. "createTableAppendColumnWithoutNullClause"... it is totally unclear what this method should do to a person writing her own adapter...

3. A formal nitpick: Cayenne is all Java 5 now. So having a StringBuffer in the method sigs is not a good idea. Appendable or StringBuilder is probably a better choice.

[ https://issues.apache.org/cayenne/browse/CAY-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Scott Anderson updated CAY-1116:
--------------------------------

Attachment: AutoIncrement-SQLite2.patch

It's not necessary - in fact, it was not even my intention to include that part of the patch here. I'm attaching the low impact version now.

For kicks, though, I would personally prefer to have the NULL clause appended in its own independent method, but that would have a much broader impact, as it would change the original methods responsibility in a non-obvious way. In any case, it doesn't have much to do with adding autoincrement to SQLite, so I think this can be tabled.

[ https://issues.apache.org/cayenne/browse/CAY-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Scott Anderson updated CAY-1116:
--------------------------------

Attachment: AutoIncrement-SQLite3.patch

Disregard #2, it's missing the field name

[ https://issues.apache.org/cayenne/browse/CAY-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13146#action_13146 ]

Scott Anderson commented on CAY-1116:
-------------------------------------

I'm 0 for 3 today. SQLiteAdapter needs setSupportsGeneratedKeys(true)

[ https://issues.apache.org/cayenne/browse/CAY-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13147#action_13147 ]

Andrus Adamchik commented on CAY-1116:
--------------------------------------

so should we close it?

[ https://issues.apache.org/cayenne/browse/CAY-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tore Halset reassigned CAY-1116:
--------------------------------

Assignee: Tore Halset (was: Andrus Adamchik)

[ https://issues.apache.org/cayenne/browse/CAY-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13149#action_13149 ]

Tore Halset commented on CAY-1116:
----------------------------------

Scott, what do you mean by "I'm 0 for 3 today"? Do you think we should apply patch number 3? Are you able to create a junit test to show that this is actually helping and working?

[ https://issues.apache.org/cayenne/browse/CAY-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150#action_13150 ]

Scott Anderson commented on CAY-1116:
-------------------------------------

No, I didn't mean it's invalid. I meant that the three patches all have flaws - the third patch needs to call setSupportsGeneratedKeys(true) instead of false in the constructor, but is otherwise functional.

Tore: A JUnit test against the new method to verify the SQL is well-formed, or to charge a real SQLite DB with a DBGenerator? If the latter, I wouldn't have a clue where to start; could you provide an example?

[ https://issues.apache.org/cayenne/browse/CAY-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Scott Anderson updated CAY-1116:
--------------------------------

Attachment: AutoIncrement-SQLite4.patch

Fixed the problem from #3, and added a JUnit test to validate SQL

[ https://issues.apache.org/cayenne/browse/CAY-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mike Kienenberger updated CAY-1116:
-----------------------------------

Attachment: (was: AutoIncrement-SQLite.patch)

[ https://issues.apache.org/cayenne/browse/CAY-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mike Kienenberger updated CAY-1116:
-----------------------------------

Attachment: (was: AutoIncrement-SQLite2.patch)

[ https://issues.apache.org/cayenne/browse/CAY-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13152#action_13152 ]

Mike Kienenberger commented on CAY-1116:
----------------------------------------

I deleted the three defective patches.

[ https://issues.apache.org/cayenne/browse/CAY-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mike Kienenberger updated CAY-1116:
-----------------------------------

Attachment: (was: AutoIncrement-SQLite3.patch)