From bb80f44a91f6d107f06cbf2030e80612f9e7bc75 Mon Sep 17 00:00:00 2001 From: Mats Kindahl Date: Fri, 15 Nov 2024 07:46:06 +0100 Subject: [PATCH] Fix issue resulting from ResultRelInfo size change Because a new member was added to `ResulRelInfo` in 17.1, our initialization of `resultRelInfo` in `ts_catalog_open_indexes` did not work correctly. Switching to use the PostgreSQL functions `CatalogOpenIndexes` and `CatalogCloseIndexes`. --- .unreleased/pr_7447 | 1 + src/ts_catalog/catalog.c | 22 ---------------------- src/ts_catalog/catalog.h | 2 -- tsl/src/compression/api.c | 6 +++--- tsl/src/compression/compression.c | 19 ++++++++++++------- tsl/src/compression/compression.h | 3 ++- 6 files changed, 18 insertions(+), 35 deletions(-) create mode 100644 .unreleased/pr_7447 diff --git a/.unreleased/pr_7447 b/.unreleased/pr_7447 new file mode 100644 index 00000000000..14e81ca198f --- /dev/null +++ b/.unreleased/pr_7447 @@ -0,0 +1 @@ +Fixes: #7447 Call InitResultRelInfo in ts_catalog_open_indexes diff --git a/src/ts_catalog/catalog.c b/src/ts_catalog/catalog.c index ad482277536..e5f211f17ed 100644 --- a/src/ts_catalog/catalog.c +++ b/src/ts_catalog/catalog.c @@ -791,28 +791,6 @@ ts_catalog_scan_all(CatalogTable table, int indexid, ScanKeyData *scankey, int n ts_scanner_scan(&scanctx); } -extern TSDLLEXPORT ResultRelInfo * -ts_catalog_open_indexes(Relation heapRel) -{ - ResultRelInfo *resultRelInfo; - - resultRelInfo = makeNode(ResultRelInfo); - resultRelInfo->ri_RangeTableIndex = 0; /* dummy */ - resultRelInfo->ri_RelationDesc = heapRel; - resultRelInfo->ri_TrigDesc = NULL; /* we don't fire triggers */ - - ExecOpenIndices(resultRelInfo, false); - - return resultRelInfo; -} - -extern TSDLLEXPORT void -ts_catalog_close_indexes(ResultRelInfo *indstate) -{ - ExecCloseIndices(indstate); - pfree(indstate); -} - /* * Copied verbatim from postgres source CatalogIndexInsert which is static * in postgres source code. diff --git a/src/ts_catalog/catalog.h b/src/ts_catalog/catalog.h index 49d77b03ccb..36d925acef4 100644 --- a/src/ts_catalog/catalog.h +++ b/src/ts_catalog/catalog.h @@ -1356,8 +1356,6 @@ extern TSDLLEXPORT void ts_catalog_update(Relation rel, HeapTuple tuple); extern TSDLLEXPORT void ts_catalog_delete_tid_only(Relation rel, ItemPointer tid); extern TSDLLEXPORT void ts_catalog_delete_tid(Relation rel, ItemPointer tid); extern TSDLLEXPORT void ts_catalog_invalidate_cache(Oid catalog_relid, CmdType operation); -extern TSDLLEXPORT ResultRelInfo *ts_catalog_open_indexes(Relation heapRel); -extern TSDLLEXPORT void ts_catalog_close_indexes(ResultRelInfo *indstate); extern TSDLLEXPORT void ts_catalog_index_insert(ResultRelInfo *indstate, HeapTuple heapTuple); bool TSDLLEXPORT ts_catalog_scan_one(CatalogTable table, int indexid, ScanKeyData *scankey, diff --git a/tsl/src/compression/api.c b/tsl/src/compression/api.c index bb615ded566..089f8db2082 100644 --- a/tsl/src/compression/api.c +++ b/tsl/src/compression/api.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -1082,10 +1083,9 @@ get_compressed_chunk_index_for_recompression(Chunk *uncompressed_chunk) CompressionSettings *settings = ts_compression_settings_get(compressed_chunk->table_id); - ResultRelInfo *indstate = ts_catalog_open_indexes(compressed_chunk_rel); + CatalogIndexState indstate = CatalogOpenIndexes(compressed_chunk_rel); Oid index_oid = get_compressed_chunk_index(indstate, settings); - - ts_catalog_close_indexes(indstate); + CatalogCloseIndexes(indstate); table_close(compressed_chunk_rel, NoLock); table_close(uncompressed_chunk_rel, NoLock); diff --git a/tsl/src/compression/compression.c b/tsl/src/compression/compression.c index 95b25e47269..2614dfb8440 100644 --- a/tsl/src/compression/compression.c +++ b/tsl/src/compression/compression.c @@ -5,6 +5,7 @@ */ #include #include +#include #include #include #include @@ -793,7 +794,7 @@ row_compressor_init(CompressionSettings *settings, RowCompressor *row_compressor ALLOCSET_DEFAULT_SIZES), .compressed_table = compressed_table, .bistate = need_bistate ? GetBulkInsertState() : NULL, - .resultRelInfo = ts_catalog_open_indexes(compressed_table), + .resultRelInfo = CatalogOpenIndexes(compressed_table), .n_input_columns = RelationGetDescr(uncompressed_table)->natts, .count_metadata_column_offset = AttrNumberGetAttrOffset(count_metadata_column_num), .compressed_values = palloc(sizeof(Datum) * num_columns_in_compressed_table), @@ -1124,7 +1125,7 @@ row_compressor_close(RowCompressor *row_compressor) { if (row_compressor->bistate) FreeBulkInsertState(row_compressor->bistate); - ts_catalog_close_indexes(row_compressor->resultRelInfo); + CatalogCloseIndexes(row_compressor->resultRelInfo); } /****************** @@ -1216,7 +1217,7 @@ build_decompressor(Relation in_rel, Relation out_rel) .out_desc = out_desc, .out_rel = out_rel, - .indexstate = ts_catalog_open_indexes(out_rel), + .indexstate = CatalogOpenIndexes(out_rel), .mycid = GetCurrentCommandId(true), .bistate = GetBulkInsertState(), @@ -1265,7 +1266,7 @@ row_decompressor_close(RowDecompressor *decompressor) { FreeBulkInsertState(decompressor->bistate); MemoryContextDelete(decompressor->per_compressed_row_ctx); - ts_catalog_close_indexes(decompressor->indexstate); + CatalogCloseIndexes(decompressor->indexstate); FreeExecutorState(decompressor->estate); detoaster_close(&decompressor->detoaster); } @@ -1550,11 +1551,15 @@ row_decompressor_decompress_row_to_table(RowDecompressor *decompressor) * insert the entire batch into one index, then into another, and so on. * Working with one index at a time gives better data access locality, * which reduces the load on shared buffers cache. + * * The normal Postgres code inserts each row into all indexes, so to do it * the other way around, we create a temporary ResultRelInfo that only - * references one index. Then we loop over indexes, and for each index we - * set it to this temporary ResultRelInfo, and insert all rows into this - * single index. + * references one index. We need to re-open the index for the relation + * here since the size of ResultRelInfo can change (this happened in 17.1) + * and a blind copy of the ResultRelInfo will not copy the correct data. + * + * Then we loop over indexes, and for each index we set it to this + * temporary ResultRelInfo, and insert all rows into this single index. */ if (decompressor->indexstate->ri_NumIndices > 0) { diff --git a/tsl/src/compression/compression.h b/tsl/src/compression/compression.h index 8e991ce9d22..0ede01bdf0e 100644 --- a/tsl/src/compression/compression.h +++ b/tsl/src/compression/compression.h @@ -6,6 +6,7 @@ #pragma once #include +#include #include #include #include @@ -130,7 +131,7 @@ typedef struct RowDecompressor TupleDesc out_desc; Relation out_rel; - ResultRelInfo *indexstate; + CatalogIndexState indexstate; EState *estate; CommandId mycid;