Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop TxMeta column from TxHistory table #4230

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

SirTyson
Copy link
Contributor

@SirTyson SirTyson commented Mar 4, 2024

Description

Rebased on #4226.

This change drops txmeta from the txhistory SQL table when BucketListDB is enabled.

Unfortunately, SQLite does not support conditional column deletion, so I've added a new persistent state field indicating whether or not txmeta is supported. This allows us to check if the txmeta column exists before executing the drop SQL transaction.

Addresses #4211.

Checklist

  • Reviewed the contributing document
  • Rebased on top of master (no merge commits)
  • Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
  • Compiles
  • Ran all tests
  • If change impacts performance, include supporting evidence per the performance document

@SirTyson SirTyson requested review from marta-lokhova and dmkozh March 4, 2024 19:57
lib/catch.hpp Outdated Show resolved Hide resolved
lib/sqlite/sqlite3.h Outdated Show resolved Hide resolved
src/util/xdrquery/XDRQueryEval.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@graydon graydon left a comment

Choose a reason for hiding this comment

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

I believe you can reflect on the table structure directly and check to see if it has a given column. For example in sqlite it's something like this:

SELECT EXISTS (
  SELECT 1
  FROM pragma_table_info('txhistory')
  WHERE name = 'txmeta'
);

and for postgres I think it's something like this:

SELECT EXISTS (
  SELECT 1
  FROM information_schema.columns 
  WHERE 
    table_name = 'txhistory' AND 
    column_name = 'txmeta'
);

@SirTyson
Copy link
Contributor Author

SirTyson commented Mar 8, 2024

I believe you can reflect on the table structure directly and check to see if it has a given column. For example in sqlite it's something like this:

SELECT EXISTS (
  SELECT 1
  FROM pragma_table_info('txhistory')
  WHERE name = 'txmeta'
);

and for postgres I think it's something like this:

SELECT EXISTS (
  SELECT 1
  FROM information_schema.columns 
  WHERE 
    table_name = 'txhistory' AND 
    column_name = 'txmeta'
);

Done

@anupsdf anupsdf changed the title Drop meta table Drop TxMeta column from TxHistory table Mar 20, 2024
@@ -709,6 +709,8 @@ ApplicationImpl::validateAndLogConfig()
{
if (mConfig.isUsingBucketListDB())
{
// Tx meta column no longer supported in BucketListDB
mPersistentState->dropTxMetaIfExists(*mDatabase);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to make this schema change in the same place than other schema changes so that people can run the upgrade-db (some people don't give the account that runs core sql access to update schema)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@marta-lokhova marta-lokhova left a comment

Choose a reason for hiding this comment

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

lgtm!

@marta-lokhova
Copy link
Contributor

r+ 31dabd1

@latobarita latobarita merged commit 75c597b into stellar:master Apr 24, 2024
15 checks passed
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.

6 participants