Skip to content

Commit

Permalink
Fix compression for tables with large oids
Browse files Browse the repository at this point in the history
When trying to compress a table with an oid > 2^31 an error about
bitmapset not allowing negative members would be raised because
we tried to store the oids in a bitmapset.
Oids should not be stored in Bitmapsets as storing Oids that way
is very inefficient and may potentially consume a lot of memory
depending on the values of the oids. Storing them as list is more
efficient instead.
  • Loading branch information
svenklemm committed Nov 4, 2023
1 parent fbc1fbb commit f6bd108
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 10 deletions.
3 changes: 3 additions & 0 deletions .unreleased/pr_6275
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fixes: #6275 Fix negative bitmapset member not allowed in compression

Thanks: @torazem for reporting an issue with compression and large oids
19 changes: 9 additions & 10 deletions tsl/src/compression/create.c
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,7 @@ get_col_info_for_attnum(Hypertable *ht, CompressColInfo *colinfo, AttrNumber att
* This is limited to foreign key constraints now
*/
static List *
validate_existing_constraints(Hypertable *ht, CompressColInfo *colinfo, Bitmapset **indexes)
validate_existing_constraints(Hypertable *ht, CompressColInfo *colinfo, List **indexes)
{
Oid relid = ht->main_table_relid;
Relation pg_constr;
Expand Down Expand Up @@ -832,7 +832,7 @@ validate_existing_constraints(Hypertable *ht, CompressColInfo *colinfo, Bitmapse
* index is a problem at this point. It potentially avoids a second
* check of an index that we have already checked. */
if (OidIsValid(form->conindid))
*indexes = bms_add_member(*indexes, form->conindid);
*indexes = lappend_oid(*indexes, form->conindid);

/*
* We check primary, unique, and exclusion constraints. Move foreign
Expand Down Expand Up @@ -923,7 +923,7 @@ validate_existing_constraints(Hypertable *ht, CompressColInfo *colinfo, Bitmapse
* by the constraint checking above.
*/
static void
validate_existing_indexes(Hypertable *ht, CompressColInfo *colinfo, Bitmapset *ignore)
validate_existing_indexes(Hypertable *ht, CompressColInfo *colinfo, List *ignore)
{
Relation pg_index;
HeapTuple htup;
Expand All @@ -948,7 +948,8 @@ validate_existing_indexes(Hypertable *ht, CompressColInfo *colinfo, Bitmapset *i
* checking. We can also skip checks below if the index is not a
* unique index. */
if (!index->indislive || !index->indisvalid || index->indisexclusion ||
index->indisprimary || !index->indisunique || bms_is_member(index->indexrelid, ignore))
index->indisprimary || !index->indisunique ||
list_member_oid(ignore, index->indexrelid))
continue;

/* Now we check that all columns of the unique index are part of the
Expand Down Expand Up @@ -1224,12 +1225,10 @@ tsl_process_compress_table(AlterTableCmd *cmd, Hypertable *ht,

/* Check if we can create a compressed hypertable with existing
* constraints and indexes. */
{
Bitmapset *indexes = NULL;
constraint_list = validate_existing_constraints(ht, &compress_cols, &indexes);
validate_existing_indexes(ht, &compress_cols, indexes);
bms_free(indexes);
}
List *indexes = NIL;
constraint_list = validate_existing_constraints(ht, &compress_cols, &indexes);
validate_existing_indexes(ht, &compress_cols, indexes);
list_free(indexes);

/* take explicit locks on catalog tables and keep them till end of txn */
LockRelationOid(catalog_get_table_id(ts_catalog_get(), HYPERTABLE), RowExclusiveLock);
Expand Down

0 comments on commit f6bd108

Please sign in to comment.