Skip to content

Commit

Permalink
Fix issue resulting from ResultRelInfo size change
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
mkindahl committed Nov 15, 2024
1 parent 910cbbf commit bb80f44
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 35 deletions.
1 change: 1 addition & 0 deletions .unreleased/pr_7447
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes: #7447 Call InitResultRelInfo in ts_catalog_open_indexes
22 changes: 0 additions & 22 deletions src/ts_catalog/catalog.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 0 additions & 2 deletions src/ts_catalog/catalog.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions tsl/src/compression/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <access/xact.h>
#include <catalog/dependency.h>
#include <catalog/index.h>
#include <catalog/indexing.h>
#include <commands/event_trigger.h>
#include <commands/tablecmds.h>
#include <commands/trigger.h>
Expand Down Expand Up @@ -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);
Expand Down
19 changes: 12 additions & 7 deletions tsl/src/compression/compression.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
#include <postgres.h>
#include <catalog/heap.h>
#include <catalog/indexing.h>
#include <catalog/pg_am.h>
#include <common/base64.h>
#include <libpq/pqformat.h>
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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);
}

/******************
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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)
{
Expand Down
3 changes: 2 additions & 1 deletion tsl/src/compression/compression.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#pragma once

#include <postgres.h>
#include <catalog/indexing.h>
#include <executor/tuptable.h>
#include <fmgr.h>
#include <lib/stringinfo.h>
Expand Down Expand Up @@ -130,7 +131,7 @@ typedef struct RowDecompressor

TupleDesc out_desc;
Relation out_rel;
ResultRelInfo *indexstate;
CatalogIndexState indexstate;
EState *estate;

CommandId mycid;
Expand Down

0 comments on commit bb80f44

Please sign in to comment.