Skip to content

fix(copy): respect auto_checkpoint setting on COPY FROM (#755) - #763

Open
adsharma wants to merge 1 commit into
mainfrom
fix/755-copy-checkpoint-toggle
Open

fix(copy): respect auto_checkpoint setting on COPY FROM (#755)#763
adsharma wants to merge 1 commit into
mainfrom
fix/755-copy-checkpoint-toggle

Conversation

@adsharma

Copy link
Copy Markdown
Contributor

Fixes #755

COPY FROM unconditionally forced a checkpoint, ignoring DBConfig::autoCheckpoint (the CALL auto_checkpoint=... setting) and DBConfig::checkpointThreshold. The forced branch in transaction_manager.cpp:169 ran first, so the auto-checkpoint branch never got a chance. The result was a large per-statement cost on every COPY that the user could not turn off.

Change (src/main/client_context.cpp, 1-line guard):

if (preparedStatement->getStatementType() == StatementType::COPY_FROM &&
    getDBConfig()->autoCheckpoint) {
    // Note: We force checkpoint for COPY_FROM statement unless the user has
    // disabled automatic checkpointing.
    Transaction::Get(*this)->setForceCheckpoint();
}

getDBConfig() is already in scope; autoCheckpoint is the boolean behind the CALL auto_checkpoint=... setting. The default behaviour is unchanged. When auto_checkpoint=false, COPY still succeeds but no longer triggers a checkpoint, so the data lives in the WAL until the next auto-checkpoint or a manual CALL db_checkpoint();.

Test (test/test_files/transaction/copy_no_auto_checkpoint.test):

  • CopyNoAutoCheckpointSmokeTestCALL auto_checkpoint=false; then COPY foo FROM (UNWIND ...), asserts success and that the setting is preserved. This is enough to detect a regression of the original bug.
  • CopyWithDefaultAutoCheckpoint — default auto_checkpoint=true, COPY succeeds.

Test would benefit from a stronger WAL-visibility assertion once a wal_replay_info-style public API is available; none exists today.

Test results: new tests pass; all 67 transaction tests continue to pass.

Perf note: a best-effort perf profile against the tinysnb COPY test shows the runtime is dominated by CSV parsing (regex_traits::transform, collate::do_transform, ~77%), not the COPY kernel. The dataset is too small to draw conclusions about the non-checkpoint COPY overhead. A larger benchmark would be the right next step for the part-2 measurement in the issue.

Previously, COPY FROM unconditionally forced a checkpoint regardless of
the auto_checkpoint setting. This change adds a guard so that
setForceCheckpoint() is only called when autoCheckpoint is true.

When auto_checkpoint=false, COPY FROM still succeeds but no longer
triggers an automatic checkpoint, allowing the user to control
checkpointing behavior via the existing configuration knobs.

Test: copy_no_auto_checkpoint.test validates both cases:
  - auto_checkpoint=false: COPY succeeds, setting preserved
  - auto_checkpoint=true (default): COPY succeeds
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.

Bug: COPY forces a checkpoint that auto_checkpoint and checkpoint_threshold cannot disable

1 participant