Conversation
ScanRelTable::initLocalStateInternal creates the per-thread ForeignRelTableScanState (or ArrowRelTableScanState / IceDiskRelTableScanState) but only calls table->initScanState in fetchNextBoundNodeBatch() and after pulling a new child tuple. The first scan() in getNextTuplesInternal therefore runs against an uninitialized scan state. The native RelTable::initScanState reads from the bound-node nodeIDVector to pick a node group, so it must run after a child tuple is in flight. External rel table backends, however, only use initScanState to set up scan-function / shared / local state and source/nodeGroupIdx — they don't need a bound node — but the first scan() still fires before any re-init call site, so we'd dereference uninitialized state and crash. ForeignRelTable::scanInternal calls tableFunc immediately, which on a null shared state throws or, on the old code, segfaults. This manifested as a regression introduced by the pg_client extension moving rel table registration to RelGroupCatalogEntry + ForeignRelTable: MATCH (a:node_person)-[k:rel_knows]->(b:node_person) crashed with 'tableFunc: sharedState is null' on the first scan. Detect 'external' by checking the dynamic type isn't the native RelTable via typeid, so any future external backend (XxxRelTable : RelTable) is handled automatically without re-enumerating here. Native RelTable behavior is unchanged: first scan() still returns false (source == NONE), then fetchNextBoundNodeBatch re-inits with the child tuple in flight.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the regression introduced by the pg_client extension moving rel table registration to RelGroupCatalogEntry + ForeignRelTable:
MATCH (a:node_person)-[k:rel_knows]->(b:node_person)was crashing withtableFunc: sharedState is nullon the first scan.Root cause
ScanRelTable::initLocalStateInternalcreates the per-thread scan state but only callstable->initScanStateinfetchNextBoundNodeBatch()and after pulling a new child tuple. The firstscan()ingetNextTuplesInternaltherefore runs against an uninitialized scan state.The native
RelTable::initScanStatereads from the bound-nodenodeIDVectorto pick a node group, so it must run after a child tuple is in flight. External rel table backends (ForeignRelTable,ArrowRelTable,IceDiskRelTable) only useinitScanStateto set up scan-function / shared / local state andsource/nodeGroupIdx— they don't need a bound node — but the firstscan()still fires before any re-init call site, so we'd dereference uninitialized state and crash.Fix
Detect "external" via
typeid(*tableInfo.table) != typeid(RelTable)and calltable->initScanStateonce after creating the per-thread scan state, so the firstscan()is well-defined. This scales to any future external backend (XxxRelTable : RelTable) without re-enumerating.Native
RelTablebehavior is unchanged: firstscan()still returns false (source == NONE), thenfetchNextBoundNodeBatchre-inits with the child tuple in flight.Companion change
The pg_client extension submodule was updated to restore
RelGroupCatalogEntryregistration forrel_*tables (so the planner has a real rel entry to anchorMATCHon) and to add a mutex around the morselcurrentOffsetintableFunc(so parallelHashJoinBuildworkers atomically claim rows). That lives in theextensionsrepo on thepg_clientbranch.Tests
Verified with the pg_client extension's Python test suite (13/13 passing, including new
test_06b_match_rel_table/test_06c_match_rel_count_parallel) and the e2e runner (newMatchOnRelTablecase). The push-down optimizer (ForeignJoinPushDownOptimizer) correctly lowersMATCH ... RETURN count(*)to a singleCOUNT_REL_TABLEoperator onrel_knows.