Skip to content

[core] Fix snapshots table failing IN filter for unknown id - #9134

Open
ArnavBalyan wants to merge 4 commits into
apache:masterfrom
ArnavBalyan:arnavb/fix-snapshots-in-filter
Open

[core] Fix snapshots table failing IN filter for unknown id#9134
ArnavBalyan wants to merge 4 commits into
apache:masterfrom
ArnavBalyan:arnavb/fix-snapshots-in-filter

Conversation

@ArnavBalyan

Copy link
Copy Markdown
Member

Purpose

  • snapshot_id in (...) statement fails when the in clause is supplied with a non existent snapshot id.
  • This is because the query reads files for each input id without checking for file presence.
  • If expired id is supplied, the query throws because snapshot manager is not checking if the snapshot exists.
  • Fix by adding filter.

Tests

  • UT

@JingsongLi

Copy link
Copy Markdown
Contributor

While IN (existing, missing) skips non-existent snapshots or schemas, the case where snapshot_id = missing is still hardcoded in new tests to throw an exception; this creates inconsistent predicate semantics for the same system table, whereas returning an empty set for non-existent IDs would be more logical for SQL queries. Additionally, the exists → read flow remains susceptible to a TOCTOU (Time-of-Check to Time-of-Use) race condition involving snapshot expiration.

@ArnavBalyan

Copy link
Copy Markdown
Member Author

While IN (existing, missing) skips non-existent snapshots or schemas, the case where snapshot_id = missing is still hardcoded in new tests to throw an exception; this creates inconsistent predicate semantics for the same system table, whereas returning an empty set for non-existent IDs would be more logical for SQL queries. Additionally, the exists → read flow remains susceptible to a TOCTOU (Time-of-Check to Time-of-Use) race condition involving snapshot expiration.

Hi @JingsongLi thanks for review, have updated the equality semantics to match IN filter, both do not fail on invalid snapshot ids, also fixed the TOCTOU concern. Thanks!

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.

2 participants