Skip to content

Rename the delete_late conflict to delete_exists#391

Merged
mason-sharp merged 1 commit intomainfrom
task/rename-SPOCK_CT_DELETE_LATE
Mar 13, 2026
Merged

Rename the delete_late conflict to delete_exists#391
mason-sharp merged 1 commit intomainfrom
task/rename-SPOCK_CT_DELETE_LATE

Conversation

@mason-sharp
Copy link
Member

"Exists" means for us that something exists that is unexpected, such as an existing row for insert_exists. In the delete_exists case, it is unexpected that a newer timestamped row exists.

"Exists" means for us that something exists that is unexpected,
such as for insert_exists. In the delete_exists case, it is
unexpected that a newer timestamped row exists.
@coderabbitai
Copy link

coderabbitai bot commented Mar 13, 2026

📝 Walkthrough

Walkthrough

The pull request renames the enum member SPOCK_CT_DELETE_LATE to SPOCK_CT_DELETE_EXISTS and refactors the conflict type naming mechanism by converting a static lookup table into a public function SpockConflictTypeName, updating all related code and test comments accordingly.

Changes

Cohort / File(s) Summary
Enum and Public API Changes
include/spock_conflict.h
Renames enum member from SPOCK_CT_DELETE_LATE to SPOCK_CT_DELETE_EXISTS (value 101 unchanged) and adds public function declaration SpockConflictTypeName(SpockConflictType t).
Conflict Type Naming Resolution
src/spock_conflict.c
Replaces static SpockConflictTypeNames lookup array with public function SpockConflictTypeName() using a switch statement. Updates all usage sites and adds handling for the renamed SPOCK_CT_DELETE_EXISTS case.
Conflict Type Usage
src/spock_apply_heap.c
Updates conflict reporting logic in spock_apply_heap_delete to use renamed enum member SPOCK_CT_DELETE_EXISTS instead of SPOCK_CT_DELETE_LATE.
Test Comment Update
tests/regress/sql/tuple_origin.sql
Updates test comment from "delete_late" to "delete_exists" to reflect the new enum naming convention.

Poem

🐰 Hoppy times with conflicts clear,
Delete-exists, no more late-fear,
Function names in public light,
Static tables bid goodnight,
Spock's conflicts now aligned just right!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: renaming SPOCK_CT_DELETE_LATE to SPOCK_CT_DELETE_EXISTS throughout the codebase.
Description check ✅ Passed The description explains the rationale for the rename, relating the semantic meaning of 'exists' to unexpected state (consistent with insert_exists), which is directly relevant to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch task/rename-SPOCK_CT_DELETE_LATE
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mason-sharp mason-sharp requested a review from danolivo March 13, 2026 01:51
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
include/spock_conflict.h (1)

79-81: Duplicate extern declarations.

These three extern variable declarations duplicate the ones at lines 42-44. While technically valid in C, this redundancy could lead to maintenance issues if one set is updated but not the other.

🔧 Suggested fix: Remove duplicate declarations
-extern int spock_conflict_resolver;
-extern int spock_conflict_log_level;
-extern bool	spock_save_resolutions;
-
 typedef enum
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/spock_conflict.h` around lines 79 - 81, Remove the duplicate extern
declarations for spock_conflict_resolver, spock_conflict_log_level, and
spock_save_resolutions by deleting the redundant extern int/ bool lines (the
second set) so only the original declarations remain; locate the duplicate block
that repeats the extern declarations for those three symbols and remove it to
avoid maintenance drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@include/spock_conflict.h`:
- Around line 79-81: Remove the duplicate extern declarations for
spock_conflict_resolver, spock_conflict_log_level, and spock_save_resolutions by
deleting the redundant extern int/ bool lines (the second set) so only the
original declarations remain; locate the duplicate block that repeats the extern
declarations for those three symbols and remove it to avoid maintenance drift.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 52053ffb-b279-4c39-a53c-d58404e66b11

📥 Commits

Reviewing files that changed from the base of the PR and between 33a26be and 5d0078f.

⛔ Files ignored due to path filters (1)
  • tests/regress/expected/tuple_origin.out is excluded by !**/*.out
📒 Files selected for processing (4)
  • include/spock_conflict.h
  • src/spock_apply_heap.c
  • src/spock_conflict.c
  • tests/regress/sql/tuple_origin.sql

@mason-sharp mason-sharp merged commit 0ae4205 into main Mar 13, 2026
10 checks passed
@mason-sharp mason-sharp deleted the task/rename-SPOCK_CT_DELETE_LATE branch March 13, 2026 21:02
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