Skip to content

Auto Increment partial index crash - #6208

Open
chands10 wants to merge 2 commits into
bloomberg:mainfrom
chands10:autoincpidx
Open

Auto Increment partial index crash#6208
chands10 wants to merge 2 commits into
bloomberg:mainfrom
chands10:autoincpidx

Conversation

@chands10

@chands10 chands10 commented Sep 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fix this crash

@testdb> create table t { schema { int a longlong b dbstore=nextsequence } keys { "b" = b { where b > 50 } } }$$
[create table t { schema { int a longlong b dbstore=nextsequence } keys { "b" = b { where b > 50 } } }] rc 0
@testdb> insert into t(a) values(1)
[insert into t(a) values(1)] failed with rc -1 cdb2_run_statement_typed_int: Cannot connect to db

comdb2: /home/schandra107/8comdb2/build-Linux/sqlite/vdbeaux.c:3791: sqlite3VdbeSerialType: Assertion `(pMem->db && pMem->db->mallocFailed) || flags&(MEM_Str|MEM_Blob)' failed.

@chands10 chands10 changed the title Autoincpidx Auto Increment partial index crash Sep 10, 2026
@chands10
chands10 marked this pull request as draft September 10, 2026 20:12
roborivers
roborivers suggested changes Sep 10, 2026
View reviewed changes

@roborivers roborivers left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cbuild submission: Error ⚠.
Regression testing: Success ✓.

The first 10 failing tests are:
analyze_partial_index_off_generated [failed with core dumped] **quarantined**
analyze [failed with core dumped] **quarantined**
reco-ddlk-sql **quarantined**
tsa
consumer_non_atomic_default_consumer_generated **quarantined**
tunables
sc_downgrade [timeout] **quarantined**
skipscan [timeout] **quarantined**

@chands10
{175743938} Don't truncate Mem.flags in the comparison opcodes
a1c38a6 
Mem.flags is 32 bits wide in this build, but OP_Eq..OP_Ge save the
original flags of both operands in u16 locals and write them back after
the comparison.  That drops everything above bit 15 - MEM_Zero,
MEM_Subtype, MEM_Xor, MEM_OpFunc and MEM_Master.

An insert into a table with an autoinc column and a partial index whose
"where" compares that column trips it.  nextsequence() leaves the
register as MEM_Master, the predicate compares it, and the restore
clears the register to no type at all.  The MakeRecord that forms the
index key then asserts in sqlite3VdbeSerialType(), or without assertions
writes a zero length blob where the master resolve marker should be:

  create table t { schema { int a longlong b dbstore=nextsequence }
                   keys { "b" = b { where b > 50 } } }$$
  insert into t(a) values(1)

OP_SeekRowid saves and restores flags the same way; widen it too.

Signed-off-by: Salil Chandra <schandra107@bloomberg.net>
@chands10
{175743938} Redo partial index predicates for autoinc columns on master
e5a234b 
The value of an autoinc column is only assigned on the master, by
set_master_columns().  The replicant that works out which keys a record
belongs in evaluates the "where" clause of every partial index before
that happens, so a predicate referencing such a column is compared
against an unresolved placeholder.  The bitmap it ships is wrong in both
directions: keys are added for rows that do not satisfy the predicate,
and rows that do satisfy it get no key at all.

Recompute ins_keys in add_record() once the sequence has been assigned,
for tables that have both a partial index and an autoinc column.  It has
to run before fixup_ireq_index_expressions(), which consults ins_keys to
decide which indexes to skip.  Tables with no autoinc column keep the
existing timing.

verify() reports the missing keys but not the spurious ones, so the test
uses a unique partial index to catch those: neither row satisfies the
predicate, so neither belongs in the index and the second insert must
not collide with the first.

Signed-off-by: Salil Chandra <schandra107@bloomberg.net>
@chands10
chands10 marked this pull request as ready for review September 16, 2026 19:25
roborivers
roborivers approved these changes Sep 16, 2026
View reviewed changes

@roborivers roborivers left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
consumer_non_atomic_default_consumer_generated **quarantined**
bind_query_plan
manual_partition
sc_downgrade [timeout] **quarantined**
sc_redo [timeout]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@roborivers roborivers roborivers approved these changes
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@chands10 @roborivers