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

stapi::refresh_ctable does not clear the initial table if no time column is provided #51

Open
mutability opened this issue Mar 17, 2016 · 5 comments
Assignees

Comments

@mutability
Copy link

If no time ("changed") column is available then refresh_ctable will reread the whole table from the DB.

However it does not appear to clear the current table contents before doing so, so deleted rows will live forever.

@resuna resuna self-assigned this Mar 27, 2016
@resuna
Copy link
Member

resuna commented Mar 27, 2016

This would appear to also be a problem if there is a time column available as well, since partial reads won't provide any information about deleted rows either. So either way deleted rows live forever.

I'm not sure this is generally resolveable at this level. What are your thoughts?

@mutability
Copy link
Author

For schemas with no change-timestamp and where rows are never deleted, the current situation works.

For schemas with no change-timestamp and where rows are deleted, the current situation does not work but could be made to work by clearing the table before re-read. This is what this specific issue is about and is probably the most common simple setup. If we support refresh-with-no-change-timestamp-column at all, we should probably support this case.

For schemas with a change-timestamp where rows are never deleted, the current situation works. A subset of this is cases where rows are never deleted at the DB level but they are flagged as inactive which is probably the most general case.

For schemas with a change-timestamp where rows are deleted, I can't see how incremental updates can be made to work; you basically have to refresh the whole table to notice deletions; to get incremental updates you need to avoid deleting rows.

@resuna
Copy link
Member

resuna commented Mar 31, 2016

I'm mostly thinking of the semantics, it should be explicitly obvious that a table is or is not fully in sync with the underlying table. Either a flag on the table to make refresh_ctable clear the table, or (maybe better) a flag on refresh_ctable to force a reload.

@resuna
Copy link
Member

resuna commented Apr 3, 2016

I've added a routine "reload_ctable" that bypasses the time check and resets the ctable before reloading it. Semantically it's otherwise identical to refresh_ctable. I kind of want to brainstorm this before refactoring it to always reset the ctable if the time can't be determined. I'd rather make the rule "always call reload_ctable if rows may be deleted" rather than having code that changes behavior if a time column is added or deleted in the definition of the stapi table.

@resuna
Copy link
Member

resuna commented Jul 11, 2016

This was merged with stapi-fixes branch. There are separate functions refresh_ctable and reload_ctable.

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

No branches or pull requests

2 participants