Cleanup: Have Form.save_records and Form.prompt_save use save_record_recursive like Query.prompt_save

[ssweber]

Writing this down so I don't forget details. 😄

• Form.save_records and Form.prompt_save loop through queries to save.
• Query.prompt_save starts at self.table, and calls save_record_recursive, which saves bottom child first and works its way back up to current table.

Looping through queries to save is an issue if a user has a child change its parent, thus requerying itself and its dependents. Any date unsaved in its dependents will be lost. Since Query.prompt_save saves the lowest first, if there is a requery, it's children are already saved.

While that is an edge-case, I like the idea of keeping the behavior consistent between all three saving methods.

Form.save_records:

• for each top-level parent, call save_record_recursive.
• Move debug lines (success/failure/no action) into Query.save_record.
• Have save_record_recursive & save_record pass on (success/failure/no action) so that Form.save_records has information for final popup message. Eg, keep passing a SAVE_NONE, until a SAVE_SUCCESS, but override with a SAVE_FAIL
• Modify save_record_recursive to pass on display_message=True

Form.prompt_save:

• for each top-level parent, call if self[q].records_changed(recursive=True)
• then for each true, call save_record_recursive. May need to modify save_record_recursive for an check_prompt_save=True flag to keep current behavior

One more small cleanup:
Query.save_record - it's flag is update_elements=True, while the rest use update=True. Consider switching to match

[PySimpleSQL]

As long as we keep things pretty much backwards compatible, I don't have a real preference on how we loop through the queries. In regards to re-parenting, It may even make more sense to do that at the SQL backend, then requery the changes in from there - it could get rid of a lot of complexity on the SS side.

Form.save_Records sounds logical. Again, the only concert would be remaining as backward compatible as possible

Yeah, Query.save_record should match the rest.

[ssweber]

Ok. When I get to this I’ll make sure to replicate any behavior!

[ssweber]

I’ll try to do this one tomorrow or Thursday.

bumped into it again, this time with virtual records.

Parent - insert virtual row
Child - insert virtual row
Child - hit save.

Depending on the query order, the child can be overwritten if the parent saves first, and requires/select by pk

so best to save lowest children first, then up :)

[PySimpleSQL]

I think it has to do with getting the primary key bu the MAX() method. Currently new records are inserted explicitly stating the primary key, where in actuality we should be letting the sequencer in the database handle the auto increment. I have a few ideas on how to handle this that I'll try out tomorrow

…

On Tue, Feb 21, 2023, 7:59 PM ssweber ***@***.***> wrote: I’ll try to do this one tomorrow or Thursday. bumped into it again, this time with virtual records. Parent - insert virtual row Child - insert virtual row Child - hit save. Depending on the query order, the child can be overwritten if the parent saves first, and requires/select by pk so best to save lowest children first, then up :) — Reply to this email directly, view it on GitHub <#71 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AQX2RESWXVUO2STAVMMBUU3WYVQF3ANCNFSM6AAAAAAU5HYBFI> . You are receiving this because you commented.Message ID: ***@***.***>

[ssweber]

Oh, like if the parent is actually assigned a different pk (eg if is autoincremented.)

are you thinking of looking at SQLITE_SEQUENCE first?

[PySimpleSQL]

Yeah, I'll start with sqlite. Using the sequencer is not without its own problems though. For example, a user creates a table and 10 entries from a backup (or even by hand like in the journal examples). In the case that we use autoincrement on our next record insert, it's not likely to pick up where the last records left off (IIRC each database handled this very differently). I have a test file I was messing with a few days back getting the sequence numbers for all 3 databases after some manual insertions and it was all over the place. So in cases like these, explicitly stating the keys is less likely to have collisions. Of course, there isnthe option to manually update the sequence internally, but that's kind of messy too. Tomorrow I'll also play with improving the current system- it should be as easy as: - adding a count_virtual() to ResultSet - on record insert, add this count to the next_pk to prevent the collisions After some experimentation, I'll pick whichever is the most stable and work it in

…

On Tue, Feb 21, 2023, 8:33 PM ssweber ***@***.***> wrote: Oh, like if the parent is actually assigned a different pk (eg if is autoincremented.) are you thinking of looking at SQLITE_SEQUENCE first? — Reply to this email directly, view it on GitHub <#71 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AQX2REUHTXHXBN67YK2YNE3WYVUF7ANCNFSM6AAAAAAU5HYBFI> . You are receiving this because you commented.Message ID: ***@***.***>

[PySimpleSQL]

I thought on this last night a bit. It occurred to me that the tests that I ran on this were before abstracting the databases. Now that I don't need a general solution, I should be able to deal with the uniqueness of each within the driver. Postgres is by far the quirkiest, but at the same time has a super neat mechanism to check out the keys before the query, which will be really useful for preventing collisions with concurrent access. SQLite is the most straight forward, typically just handing out the next highest value from whatever the max is. Concurrent access will be a little trickier but I think I have a plan for that MySql kind of sits in between the two. It normally picks the next highest, but it isn't guaranteed, as it will reuse old keys in some instances. Fun stuff

…

On Tue, Feb 21, 2023, 8:55 PM Jon Decker ***@***.***> wrote: Yeah, I'll start with sqlite. Using the sequencer is not without its own problems though. For example, a user creates a table and 10 entries from a backup (or even by hand like in the journal examples). In the case that we use autoincrement on our next record insert, it's not likely to pick up where the last records left off (IIRC each database handled this very differently). I have a test file I was messing with a few days back getting the sequence numbers for all 3 databases after some manual insertions and it was all over the place. So in cases like these, explicitly stating the keys is less likely to have collisions. Of course, there isnthe option to manually update the sequence internally, but that's kind of messy too. Tomorrow I'll also play with improving the current system- it should be as easy as: - adding a count_virtual() to ResultSet - on record insert, add this count to the next_pk to prevent the collisions After some experimentation, I'll pick whichever is the most stable and work it in On Tue, Feb 21, 2023, 8:33 PM ssweber ***@***.***> wrote: > Oh, like if the parent is actually assigned a different pk (eg if is > autoincremented.) > > are you thinking of looking at SQLITE_SEQUENCE first? > > — > Reply to this email directly, view it on GitHub > <#71 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AQX2REUHTXHXBN67YK2YNE3WYVUF7ANCNFSM6AAAAAAU5HYBFI> > . > You are receiving this because you commented.Message ID: > ***@***.***> >

[ssweber]

Also, if this makes things simpler for you

• on insert we could check if parent current row is virtual, and disallow child inserts before saving parent.

[ssweber]

I know they have to worry about performance, their implementation and all that, but I always think it’s funny when databases say “typically” in their documentation.

[ssweber]

#86 completes this