Обсуждение: BUG #18363: Assert !ReindexIsProcessingIndex falsified with expression index over select from table
BUG #18363: Assert !ReindexIsProcessingIndex falsified with expression index over select from table
От
PG Bug reporting form
Дата:
The following bug has been logged on the website: Bug reference: 18363 Logged by: Alexander Lakhin Email address: exclusion@gmail.com PostgreSQL version: 16.2 Operating system: Ubuntu 22.04 Description: The following script: CREATE TABLE t(i INT PRIMARY KEY); CREATE FUNCTION f(i int) RETURNS int IMMUTABLE LANGUAGE SQL RETURN (SELECT i FROM t WHERE i = $1); CREATE INDEX ON t(f(i)); INSERT INTO t VALUES (1); REINDEX INDEX t_f_idx; triggers an assertion failure with the stack trace: TRAP: failed Assert("!ReindexIsProcessingIndex(RelationGetRelid(indexRelation))"), File: "indexam.c", Line: 778, PID: 1339223 ExceptionalCondition at assert.c:52:13 index_getprocid at indexam.c:817:1 get_relation_info at plancat.c:272:24 build_simple_rel at relnode.c:379:5 add_base_rels_to_query at initsplan.c:166:10 add_base_rels_to_query at initsplan.c:173:3 query_planner at planmain.c:180:2 grouping_planner at planner.c:1495:17 subquery_planner at planner.c:1070:2 make_subplan at subselect.c:221:12 process_sublinks_mutator at subselect.c:1950:10 expression_tree_mutator_impl at nodeFuncs.c:3302:5 process_sublinks_mutator at subselect.c:2052:9 expression_tree_mutator_impl at nodeFuncs.c:3389:12 process_sublinks_mutator at subselect.c:2052:9 SS_process_sublinks at subselect.c:1924:1 preprocess_expression at planner.c:1171:10 subquery_planner at planner.c:810:20 standard_planner at planner.c:413:9 planner at planner.c:282:9 pg_plan_query at postgres.c:904:9 init_execution_state at functions.c:497:12 init_sql_fcache at functions.c:797:21 fmgr_sql at functions.c:1097:40 ExecInterpExpr at execExprInterp.c:735:7 ExecInterpExprStillValid at execExprInterp.c:1871:1 MemoryContextSwitchTo at palloc.h:142:23 (inlined by) ExecEvalExprSwitchContext at executor.h:356:2 (inlined by) FormIndexDatum at index.c:2774:13 heapam_index_build_range_scan at heapam_handler.c:1660:7 table_index_build_scan at tableam.h:1781:9 (inlined by) _bt_spools_heapscan at nbtsort.c:483:15 btbuild at nbtsort.c:329:14 index_build at index.c:3042:10 reindex_index at index.c:3763:2 ReindexIndex at indexcmds.c:2793:1 ExecReindex at indexcmds.c:2741:1 standard_ProcessUtility at utility.c:965:4 ProcessUtility at utility.c:530:3 PortalRunUtility at pquery.c:1168:2 PortalRunMulti at pquery.c:1315:5 PortalRun at pquery.c:795:5 exec_simple_query at postgres.c:1282:3 PostgresMain at postgres.c:4641:27 report_fork_failure_to_client at postmaster.c:4242:1 BackendStartup at postmaster.c:4199:22 ServerLoop at postmaster.c:1788:6 BackgroundWorkerInitializeConnection at postmaster.c:5604:1 main at main.c:185:3 With a non-assertion-enabled build it fails with: ERROR: could not read block 0 in file "base/16384/16392": read only 0 of 8192 bytes CONTEXT: SQL function "f" during startup Surely, such an index will not work correctly anyway, but may be it makes sense to replace that Assert with ereport(ERROR).
PG Bug reporting form <noreply@postgresql.org> writes: > The following script: > CREATE TABLE t(i INT PRIMARY KEY); > CREATE FUNCTION f(i int) RETURNS int IMMUTABLE LANGUAGE SQL > RETURN (SELECT i FROM t WHERE i = $1); > CREATE INDEX ON t(f(i)); > INSERT INTO t VALUES (1); > REINDEX INDEX t_f_idx; > triggers an assertion failure with the stack trace: > ... > With a non-assertion-enabled build it fails with: > ERROR: could not read block 0 in file "base/16384/16392": read only 0 of > 8192 bytes > CONTEXT: SQL function "f" during startup > Surely, such an index will not work correctly anyway, but may be it makes > sense to replace that Assert with ereport(ERROR). Yeah, I guess so. We periodically get reports of the non-assert failure, and this would let us issue a more on-point error message. I'm slightly worried about the extra cost, but typically pendingReindexedIndexes should be empty or at least short, so it's probably negligible. regards, tom lane
On Sun, Feb 25, 2024 at 03:37:08PM -0500, Tom Lane wrote: > Yeah, I guess so. We periodically get reports of the non-assert > failure, and this would let us issue a more on-point error message. > I'm slightly worried about the extra cost, but typically > pendingReindexedIndexes should be empty or at least short, so it's > probably negligible. An index expression that calls a function doing a scan of its parent table is a funky case for sure, but perhaps f5a465f1a074 would have been better with a test? -- Michael
Вложения
Michael Paquier <michael@paquier.xyz> writes: > An index expression that calls a function doing a scan of its parent > table is a funky case for sure, but perhaps f5a465f1a074 would have > been better with a test? I wasn't excited about having a test case, but feel free to add one if you think it's worth the cycles. regards, tom lane
Re: BUG #18363: Assert !ReindexIsProcessingIndex falsified with expression index over select from table
От
Alexander Lakhin
Дата:
Hello Tom and Michael, 25.02.2024 23:37, Tom Lane wrote: >> Surely, such an index will not work correctly anyway, but may be it makes >> sense to replace that Assert with ereport(ERROR). > Yeah, I guess so. We periodically get reports of the non-assert > failure, and this would let us issue a more on-point error message. > I'm slightly worried about the extra cost, but typically > pendingReindexedIndexes should be empty or at least short, so it's > probably negligible. Thank you for fixing that! I've also got the same error (but not the assert) with CREATE INDEX: CREATE TABLE t(i int PRIMARY KEY); CREATE FUNCTION f(c int) RETURNS INT IMMUTABLE LANGUAGE SQL AS 'SELECT i FROM t WHERE i = $1'; INSERT INTO t VALUES (1); CREATE INDEX ON t(f(i)); ERROR: could not read block 0 in file "base/16384/16391": read only 0 of 8192 bytes CONTEXT: SQL function "f" during startup It looks like currentlyReindexedIndex == 0 in this case, so ReindexIsProcessingIndex() doesn't guard against get_relation_info() -> _bt_getrootheight() -> _bt_getbuf() -> ReadBuffer() -> ... -> mdread(). Best regards, Alexander
On Mon, Feb 26, 2024 at 09:00:01AM +0300, Alexander Lakhin wrote: > It looks like currentlyReindexedIndex == 0 in this case, so > ReindexIsProcessingIndex() doesn't guard against get_relation_info() -> > _bt_getrootheight() -> _bt_getbuf() -> ReadBuffer() -> ... -> mdread(). Fun. The concurrent case is actually able to work because it looks like the basic definition of the relation exists with its relfilenode. I was looking at that, and it seems to me that the point is not that the index is being reindexed; the point is that we want to prevent access to the index itself while it being built. And that's something that can happen for a reindex as much as a new index. It would be possible to paint an equivalent of SetReindexProcessing() in index_create() for index_build() where a trace of the index OID getting built is kept around, and it would be possible to trigger the same error as when doing a reindex. Hence, if we were to do that, the current ReindexIs*() routines maintaining the list of the indexes being built across transaction states are a bit misnamed, and the error messages would be partially incorrect. I am not sure if this is worth bothering beyond HEAD, or worth bothering at all, but seeing 940489b467 it looks like we do bother even for stable branches. -- Michael