Skip to content

Commit

Permalink
Add support for ALTER COLUMN SET NOT NULL
Browse files Browse the repository at this point in the history
Since `SET NOT NULL` will only do a full table scan of the table to
verify that there are no nulls, this should work without having to
modify any data in the table. However, it does not work for chunks
using the `heap` table access method if they are compressed since it
uses a normal table scan, so adding a check for this.
  • Loading branch information
mkindahl committed Nov 20, 2024
1 parent 53299b9 commit b1b2380
Show file tree
Hide file tree
Showing 10 changed files with 302 additions and 74 deletions.
57 changes: 49 additions & 8 deletions src/process_utility.c
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ check_alter_table_allowed_on_ht_with_compression(Hypertable *ht, AlterTableStmt
case AT_ReAddStatistics:
case AT_SetCompression:
case AT_DropNotNull:
case AT_SetNotNull:
#if PG15_GE
case AT_SetAccessMethod:
#endif
Expand Down Expand Up @@ -2563,11 +2564,50 @@ process_altertable_validate_constraint_end(Hypertable *ht, AlterTableCmd *cmd)
foreach_chunk(ht, validate_hypertable_constraint, cmd);
}

/*
* Validate that SET NOT NULL is ok for this chunk.
*
* Throws an error if SET NOT NULL on this chunk is not allowed, right now,
* SET NOT NULL is allowed on chunks that are either a fully decompressed, or
* are using the Hypercore table access method.
*/
static void
validate_set_not_null(Hypertable *ht, Oid chunk_relid, void *arg)
{
Chunk *chunk = ts_chunk_get_by_relid(chunk_relid, true);
if (ts_chunk_is_compressed(chunk) && !ts_is_hypercore_am(chunk->amoid))
{
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("operation not supported on compressed chunks not using the "
"\"hypercore\" table access method"),
errdetail("Chunk %s.%s is using the heap table access method and has compressed "
"data.",
NameStr(chunk->fd.schema_name),
NameStr(chunk->fd.table_name)),
errhint("Either decompress all chunks of the hypertable or use \"ALTER TABLE "
"%s.%s SET ACCESS METHOD hypercore\" on all chunks to change access "
"method.",
NameStr(chunk->fd.schema_name),
NameStr(chunk->fd.table_name))));
}
}

/*
* This function checks that we are not dropping NOT NULL from bad columns and
* that all chunks support the modification.
*/
static void
process_altertable_drop_not_null(Hypertable *ht, AlterTableCmd *cmd)
process_altertable_alter_not_null_start(Hypertable *ht, AlterTableCmd *cmd)
{
int i;

if (cmd->subtype == AT_SetNotNull)
foreach_chunk(ht, validate_set_not_null, cmd);

if (cmd->subtype != AT_DropNotNull)
return;

for (i = 0; i < ht->space->num_dimensions; i++)
{
Dimension *dim = &ht->space->dimensions[i];
Expand Down Expand Up @@ -3803,9 +3843,10 @@ process_altertable_start_table(ProcessUtilityArgs *args)
verify_constraint_hypertable(ht, cmd->def);
}
break;
case AT_SetNotNull:
case AT_DropNotNull:
if (ht != NULL)
process_altertable_drop_not_null(ht, cmd);
process_altertable_alter_not_null_start(ht, cmd);
break;
case AT_AddColumn:
#if PG16_LT
Expand Down Expand Up @@ -4187,6 +4228,8 @@ process_altertable_end_subcmd(Hypertable *ht, Node *parsetree, ObjectAddress *ob
case AT_DropCluster:
foreach_chunk(ht, process_altertable_chunk, cmd);
break;
case AT_SetNotNull:
case AT_DropNotNull:
case AT_SetRelOptions:
case AT_ResetRelOptions:
case AT_ReplaceRelOptions:
Expand All @@ -4213,8 +4256,6 @@ process_altertable_end_subcmd(Hypertable *ht, Node *parsetree, ObjectAddress *ob
case AT_SetStorage:
case AT_ColumnDefault:
case AT_CookedColumnDefault:
case AT_SetNotNull:
case AT_DropNotNull:
case AT_AddOf:
case AT_DropOf:
case AT_AddIdentity:
Expand Down Expand Up @@ -4494,8 +4535,8 @@ process_reassign_owned_start(ProcessUtilityArgs *args)
Oid newrole_oid = get_rolespec_oid(stmt->newrole, false);
HeapTuple tuple = ts_scanner_fetch_heap_tuple(ti, false, &should_free);

/* We do not need to check privileges here since ReassignOwnedObjects() will check the
* privileges and error out if they are not correct. */
/* We do not need to check privileges here since ReassignOwnedObjects() will check
* the privileges and error out if they are not correct. */
ts_bgw_job_update_owner(ti->scanrel, tuple, ts_scanner_get_tupledesc(ti), newrole_oid);

if (should_free)
Expand Down Expand Up @@ -4631,8 +4672,8 @@ process_create_stmt(ProcessUtilityArgs *args)
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("hypercore access method not supported on \"%s\"", stmt->relation->relname),
errdetail("The hypercore access method is only supported for hypertables."),
errhint("It does not make sense to set the default access method for all tables "
"to \"%s\" since it is only supported for hypertables.",
errhint("It does not make sense to set the default access method for all "
"tables to \"%s\" since it is only supported for hypertables.",
TS_HYPERCORE_TAM_NAME));

return DDL_CONTINUE;
Expand Down
1 change: 1 addition & 0 deletions tsl/src/hypercore/utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <catalog/pg_class.h>
#include <commands/defrem.h>
#include <nodes/makefuncs.h>
#include <postgres_ext.h>
#include <storage/lmgr.h>
#include <storage/lockdefs.h>
#include <utils/builtins.h>
Expand Down
26 changes: 25 additions & 1 deletion tsl/test/expected/compression_ddl.out
Original file line number Diff line number Diff line change
Expand Up @@ -2591,4 +2591,28 @@ SELECT count(*) FROM test2 WHERE i IS NULL;
1
(1 row)

SET client_min_messages = NOTICE;
SELECT count(compress_chunk(ch)) FROM show_chunks('test2') ch;
count
-------
28
(1 row)

SELECT count(*) FROM test2 WHERE i IS NULL;
count
-------
1
(1 row)

\set ON_ERROR_STOP 0
ALTER TABLE test2 ALTER COLUMN i SET NOT NULL;
ERROR: operation not supported on compressed chunks not using the "hypercore" table access method
DELETE FROM test2 WHERE i IS NULL;
SELECT count(*) FROM test2 WHERE i IS NULL;
count
-------
0
(1 row)

ALTER TABLE test2 ALTER COLUMN i SET NOT NULL;
ERROR: operation not supported on compressed chunks not using the "hypercore" table access method
\set ON_ERROR_STOP 1
2 changes: 1 addition & 1 deletion tsl/test/expected/compression_errors-14.out
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ DETAIL: Cannot drop column that is a hypertable partitioning (space or time) di
ALTER TABLE foo DROP COLUMN b;
ERROR: cannot drop orderby or segmentby column from a hypertable with compression enabled
ALTER TABLE foo ALTER COLUMN t SET NOT NULL;
ERROR: operation not supported on hypertables that have compression enabled
ERROR: column "t" of relation "_hyper_10_2_chunk" contains null values
ALTER TABLE foo RESET (timescaledb.compress);
ERROR: compression options cannot be reset
--can add constraints as long as no data is compressed
Expand Down
2 changes: 1 addition & 1 deletion tsl/test/expected/compression_errors-15.out
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ DETAIL: Cannot drop column that is a hypertable partitioning (space or time) di
ALTER TABLE foo DROP COLUMN b;
ERROR: cannot drop orderby or segmentby column from a hypertable with compression enabled
ALTER TABLE foo ALTER COLUMN t SET NOT NULL;
ERROR: operation not supported on hypertables that have compression enabled
ERROR: column "t" of relation "_hyper_10_2_chunk" contains null values
ALTER TABLE foo RESET (timescaledb.compress);
ERROR: compression options cannot be reset
--can add constraints as long as no data is compressed
Expand Down
2 changes: 1 addition & 1 deletion tsl/test/expected/compression_errors-16.out
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ DETAIL: Cannot drop column that is a hypertable partitioning (space or time) di
ALTER TABLE foo DROP COLUMN b;
ERROR: cannot drop orderby or segmentby column from a hypertable with compression enabled
ALTER TABLE foo ALTER COLUMN t SET NOT NULL;
ERROR: operation not supported on hypertables that have compression enabled
ERROR: column "t" of relation "_hyper_10_2_chunk" contains null values
ALTER TABLE foo RESET (timescaledb.compress);
ERROR: compression options cannot be reset
--can add constraints as long as no data is compressed
Expand Down
2 changes: 1 addition & 1 deletion tsl/test/expected/compression_errors-17.out
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ DETAIL: Cannot drop column that is a hypertable partitioning (space or time) di
ALTER TABLE foo DROP COLUMN b;
ERROR: cannot drop orderby or segmentby column from a hypertable with compression enabled
ALTER TABLE foo ALTER COLUMN t SET NOT NULL;
ERROR: operation not supported on hypertables that have compression enabled
ERROR: column "t" of relation "_hyper_10_2_chunk" contains null values
ALTER TABLE foo RESET (timescaledb.compress);
ERROR: compression options cannot be reset
--can add constraints as long as no data is compressed
Expand Down
Loading

0 comments on commit b1b2380

Please sign in to comment.