Skip to content

Allow merge regions to cross pages - #158

Merged
Shillaker merged 7 commits into
masterfrom
cross-page-merge-regions
Oct 19, 2021
Merged

Allow merge regions to cross pages#158
Shillaker merged 7 commits into
masterfrom
cross-page-merge-regions

Conversation

@Shillaker

@Shillaker Shillaker commented Oct 18, 2021

Copy link
Copy Markdown
Collaborator

Previously merge regions could only be up to one page long. This is inconvenient and non-obvious to users of the API.

Changes:

  • Allow merge regions to extend over multiple pages.
  • Add tests and bug fixes around edge cases that weren't previously caught.
  • Add trace logging to diffing algorithm to aid debugging.

@Shillaker
Shillaker marked this pull request as draft October 18, 2021 10:38
@Shillaker
Shillaker marked this pull request as ready for review October 18, 2021 16:47
@Shillaker Shillaker self-assigned this Oct 18, 2021
Comment thread src/util/snapshot.cpp
SPDLOG_ERROR("Cannot convert snapshot data type to string: {}", dt);
throw std::runtime_error("Cannot convert data type to string");
}
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if there's a nicer way to do this. Could perhaps be done with a macro, but not sure it would be any clearer or take up fewer lines.

@Shillaker
Shillaker merged commit 0b251c1 into master Oct 19, 2021
@Shillaker
Shillaker deleted the cross-page-merge-regions branch October 19, 2021 14:17
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