Skip to content

Conversation

simonw
Copy link
Owner

@simonw simonw commented May 9, 2025

Refs:

TODO:

  • Update existing failing tests
  • Get updates_since() working correctly

@simonw
Copy link
Owner Author

simonw commented May 9, 2025

I removed this bit of a test:

    # Now update a column that's part of the compound primary key
    time.sleep(0.1)
    if pks == ["id", "name"]:
        db[table_name].update((2, "Pancakes"), {"name": "Pancakes the corgi"})
        # This should have renamed the row in the chronicle table as well
        renamed_row = db[chronicle_table].get((2, "Pancakes the corgi"))
        assert renamed_row["updated_ms"] > version
    else:
        # Update single primary key
        db[table_name].update(2, {"id": 4})
        # This should have renamed the row in the chronicle table as well
        renamed_row = db[chronicle_table].get(4)
        assert renamed_row["updated_ms"] > version

@simonw
Copy link
Owner Author

simonw commented May 9, 2025

This code here:

sql = textwrap.dedent(
f"""
SELECT {select}
FROM {chron} AS c
LEFT JOIN "{table_name}" AS t
ON {join}
WHERE c.__version > ?
ORDER BY c.__version
LIMIT {batch_size}
"""
).strip()
while True:
rows = cur.execute(sql, (since,)).fetchall()
if not rows:
break
for r in rows:
since = r["__version"]

Assumes that __version is incremented for every inserted row. My current initial table population code sets __version to 1 for every existing record, which is incompatible with that pagination mechanism:

# 2) Seed chronicle table with existing rows
cols_insert = (
", ".join(f'"{col}"' for col in primary_key_names)
+ ", __added_ms, __updated_ms, __version, __deleted"
)
cols_select = (
", ".join(f'"{col}"' for col in primary_key_names)
+ f", {current_timestamp_expr} AS __added_ms, {current_timestamp_expr} AS __updated_ms, 1 AS __version, 0 AS __deleted"
)

@simonw
Copy link
Owner Author

simonw commented May 9, 2025

I switched to a window function to assign those incrementing version numbers as part of that initial insert. https://chatgpt.com/share/681e5184-0448-8006-bd11-67be8379161e

@simonw simonw merged commit 448c7f8 into main May 9, 2025
10 checks passed
@simonw simonw deleted the upsert branch May 9, 2025 19:25
simonw added a commit that referenced this pull request May 9, 2025
simonw added a commit that referenced this pull request May 9, 2025
@simonw simonw mentioned this pull request May 9, 2025
simonw added a commit that referenced this pull request May 9, 2025
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

Successfully merging this pull request may close these issues.

1 participant