Skip to content

Bug: Yapper DB operation inconsistencies #225

@ngjunsiang

Description

@ngjunsiang

From PR #224 :

Review comment

# Notify subscriptions
self._executemany(
"INSERT INTO unread (client_id, event_id) VALUES (?, ?)",
[(client_id, event_id) for client_id in subscriptions]
)

The SQL query uses SQLite parameter style (?) but the event_id variable is a dictionary. Line 108 extracts event_id as result["fetchall"][0] which returns the entire row dictionary, but the query expects just the ID value. Should be event_id["id"].

Review comment

# Notify subscriptions
if subscriptions:
self._executemany(
"INSERT INTO unread (client_id, event_id) VALUES (%s, %s)",
[(client_id, event_id) for client_id in subscriptions]
)

Missing column label in the INSERT statement. The table schema on line 97-108 shows unread table has (client_id, label, event_id) as primary key, but the INSERT only provides client_id and event_id.

                "INSERT INTO unread (client_id, label, event_id) VALUES (%s, %s, %s)",
                [(client_id, label, event_id) for client_id in subscriptions]

Review comment

# Notify subscriptions
if subscriptions:
self._executemany(
"INSERT INTO unread (client_id, event_id) VALUES (%s, %s)",
[(client_id, event_id) for client_id in subscriptions]
)

The query attempts to insert event_id for each subscription, but event_id is used in the list comprehension variable name while also being the extracted ID from line 118. This creates a variable name collision that could lead to incorrect values being inserted.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinghelp wantedExtra attention is needed

    Type

    Projects

    Status

    Open

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions