Implement Table per ACS Snapshot - #6515
Open
OriolMunoz-da wants to merge 15 commits into
Open
OriolMunoz-da wants to merge 15 commits into
OriolMunoz-da wants to merge 15 commits into
Conversation
[ci] Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>
OriolMunoz-da
commented
Jul 22, 2026
Comment on lines
+4
to
+8
| -- these values won't be set anymore | ||
| drop constraint acs_snapshot_first_row_id_fkey, | ||
| drop constraint acs_snapshot_last_row_id_fkey, | ||
| alter column first_row_id drop not null, | ||
| alter column last_row_id drop not null, |
Contributor
Author
There was a problem hiding this comment.
I didn't find mention of this in the design, but it's necessary
OriolMunoz-da
commented
Jul 22, 2026
Comment on lines
+2
to
+3
| -- the name 'table_name' is not reserved, but it is a PostgreSQL keyword, so we use a different name to avoid confusion | ||
| add column data_table_name text default null, |
Contributor
Author
There was a problem hiding this comment.
this seems like a sensible deviation from the design
Contributor
There was a problem hiding this comment.
yes that's fine
OriolMunoz-da
commented
Jul 22, 2026
Contributor
Author
There was a problem hiding this comment.
Included the minimal changes to validate:
legacy snapshots remain readable.
OriolMunoz-da
commented
Jul 22, 2026
| @@ -0,0 +1,14 @@ | |||
| alter table acs_snapshot | |||
Contributor
Author
There was a problem hiding this comment.
by the way: the table acs_incremental_snapshot probably needs the table name for the same reason this one does
OR
if it's derived from target_record_time, then so can acs_snapshot and therefore we don't need to add the column
wdyt?
Contributor
Author
There was a problem hiding this comment.
and also: this makes me consider not merging this to main until the rest of E1 is done, because I expect other surprises to popup
Contributor
There was a problem hiding this comment.
sounds good to wait for merging
OriolMunoz-da
marked this pull request as ready for review
July 22, 2026 13:24
ray-roestenburg-da
approved these changes
Jul 22, 2026
ray-roestenburg-da
left a comment
ray-roestenburg-da
left a comment
Contributor
There was a problem hiding this comment.
LGTM, but as you say, maybe better to wait with merging
[ci] Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>
Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>
* tentative SQL Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * support ACS snapshots per table Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * fixes Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * add v2 tests and get it to work [ci] Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] refactor and remove TODOs Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * fix/add todos [ci] Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] scalafmt Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] prevent tablename collisions Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * fix imports Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * ddl lock on drop Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * distinct on Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * Move migration Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * fixes Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * properly fix the query which probably breaks indexes Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * whatever Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] make it compile Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] fixes Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] new table definition Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * update query Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * review comment: use table names Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * update metric tracking Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * versioning Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * use ddl lock Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] run Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] add test for QueryAcsSnapshotPaginationTokenTest serde Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * add todo #7438 Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> --------- Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>
* make tests pass Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] cleanup Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] scalafmt Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * enable in all tests Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] fix concurrent runs Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] scalafmt Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] stupid imports Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] fix config Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] fix sanity check forcing resume ACS snapshot trigger Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * [ci] no need to drop tables what was i even thinking Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> * Add ACS snapshot table indexes (#6834) --------- Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com> --------- Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>
Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>
Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>
OriolMunoz-da
changed the title
Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>
OriolMunoz-da
enabled auto-merge (squash)
September 28, 2026 13:33
Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6262
Fixes #6263
Fixes #6264