Skip to content

Commit

Permalink
Support ALTER COLUMN SET NOT NULL on compressed chunks
Browse files Browse the repository at this point in the history
This patch adds support for ALTER TABLE ALTER COLUMN SET NOT NULL
to compressed chunks. The statement will be allowed when no NULL
values for the specific column are present in compressed chunks.
  • Loading branch information
svenklemm committed Feb 14, 2025
1 parent 55fec68 commit f7bc2fe
Show file tree
Hide file tree
Showing 13 changed files with 301 additions and 36 deletions.
1 change: 1 addition & 0 deletions .unreleased/pr_7707
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Implements: #7707 Support ALTER COLUMN SET NOT NULL on compressed chunks
5 changes: 5 additions & 0 deletions sql/pre_install/types.functions.sql
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ CREATE OR REPLACE FUNCTION _timescaledb_functions.compressed_data_info(_timescal
LANGUAGE C STRICT IMMUTABLE
AS '@MODULE_PATHNAME@', 'ts_compressed_data_info';

CREATE OR REPLACE FUNCTION _timescaledb_functions.compressed_data_has_nulls(_timescaledb_internal.compressed_data)
RETURNS BOOL
LANGUAGE C STRICT IMMUTABLE
AS '@MODULE_PATHNAME@', 'ts_compressed_data_has_nulls';

CREATE OR REPLACE FUNCTION _timescaledb_functions.dimension_info_in(cstring)
RETURNS _timescaledb_internal.dimension_info
LANGUAGE C STRICT IMMUTABLE
Expand Down
5 changes: 5 additions & 0 deletions sql/updates/latest-dev.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
ALTER TABLE _timescaledb_internal.bgw_job_stat_history
ALTER COLUMN succeeded DROP NOT NULL,
ALTER COLUMN succeeded DROP DEFAULT;

CREATE FUNCTION _timescaledb_functions.compressed_data_has_nulls(_timescaledb_internal.compressed_data)
RETURNS BOOL
LANGUAGE C STRICT IMMUTABLE
AS '@MODULE_PATHNAME@', 'ts_update_placeholder';
3 changes: 3 additions & 0 deletions sql/updates/reverse-dev.sql
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@ UPDATE _timescaledb_internal.bgw_job_stat_history SET succeeded = FALSE WHERE su
ALTER TABLE _timescaledb_internal.bgw_job_stat_history
ALTER COLUMN succeeded SET NOT NULL,
ALTER COLUMN succeeded SET DEFAULT FALSE;

DROP FUNCTION IF EXISTS _timescaledb_functions.compressed_data_has_nulls(_timescaledb_internal.compressed_data);

1 change: 1 addition & 0 deletions src/cross_module_fn.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ CROSSMODULE_WRAPPER(compressed_data_recv);
CROSSMODULE_WRAPPER(compressed_data_in);
CROSSMODULE_WRAPPER(compressed_data_out);
CROSSMODULE_WRAPPER(compressed_data_info);
CROSSMODULE_WRAPPER(compressed_data_has_nulls);
CROSSMODULE_WRAPPER(deltadelta_compressor_append);
CROSSMODULE_WRAPPER(deltadelta_compressor_finish);
CROSSMODULE_WRAPPER(gorilla_compressor_append);
Expand Down
1 change: 1 addition & 0 deletions src/cross_module_fn.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ typedef struct CrossModuleFunctions
PGFunction compressed_data_in;
PGFunction compressed_data_out;
PGFunction compressed_data_info;
PGFunction compressed_data_has_nulls;
bool (*process_compress_table)(AlterTableCmd *cmd, Hypertable *ht,
WithClauseResult *with_clause_options);
void (*process_altertable_cmd)(Hypertable *ht, const AlterTableCmd *cmd);
Expand Down
113 changes: 79 additions & 34 deletions src/process_utility.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <commands/trigger.h>
#include <commands/user.h>
#include <commands/vacuum.h>
#include <executor/spi.h>
#include <miscadmin.h>
#include <nodes/makefuncs.h>
#include <nodes/nodes.h>
Expand Down Expand Up @@ -2570,57 +2571,101 @@ process_altertable_validate_constraint_end(Hypertable *ht, AlterTableCmd *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))));
StringInfoData command;
AlterTableCmd *cmd = (AlterTableCmd *) arg;
Chunk *comp_chunk = ts_chunk_get_by_id(chunk->fd.compressed_chunk_id, true);

initStringInfo(&command);
appendStringInfo(&command,
"SELECT EXISTS(SELECT FROM %s.%s WHERE ",
quote_identifier(NameStr(comp_chunk->fd.schema_name)),
quote_identifier(NameStr(comp_chunk->fd.table_name)));

CompressionSettings *settings = ts_compression_settings_get(comp_chunk->table_id);
if (ts_array_is_member(settings->fd.segmentby, cmd->name))
{
/* For segmentby we can check directly whether NULLS are present */
appendStringInfo(&command, "%s IS NULL", quote_identifier(cmd->name));
}
else
{
/* For other columns we need to check whether we have a DEFAULT in which
* case NULL as column value would be fine.
* */
HeapTuple atttuple = SearchSysCacheAttName(ht->main_table_relid, cmd->name);
Form_pg_attribute attform = ((Form_pg_attribute) GETSTRUCT(atttuple));

if (attform->atthasdef)
appendStringInfo(&command,
"%s IS NOT NULL AND "
"_timescaledb_functions.compressed_data_has_nulls(%s)",
quote_identifier(cmd->name),
quote_identifier(cmd->name));
else
appendStringInfo(&command,
"%s IS NULL OR "
"_timescaledb_functions.compressed_data_has_nulls(%s)",
quote_identifier(cmd->name),
quote_identifier(cmd->name));
}
appendStringInfo(&command, ")");

if (SPI_connect() != SPI_OK_CONNECT)
elog(ERROR, "could not connect to SPI");

int res = SPI_execute(command.data, true /* read_only */, 0 /*count*/);

if (res < 0)
ereport(ERROR,
(errcode(ERRCODE_INTERNAL_ERROR),
(errmsg("could not verify presence of NULL values on \"%s\"",
get_rel_name(chunk_relid)))));

bool isnull;
Datum has_nulls = SPI_getbinval(SPI_tuptable->vals[0], SPI_tuptable->tupdesc, 1, &isnull);

if (isnull || DatumGetBool(has_nulls))
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
(errmsg("column \"%s\" of relation \"%s\" contains null values",
cmd->name,
get_rel_name(chunk_relid)))));

res = SPI_finish();
if (res != SPI_OK_FINISH)
elog(ERROR, "SPI_finish failed: %s", SPI_result_code_string(res));
}
}

/*
* This function checks that we are not dropping NOT NULL from bad columns and
* that all chunks support the modification.
* This function checks that we are not dropping NOT NULL from partitioning columns and
* that no NULL data is present when adding NOT NULL constraint.
*/
static void
process_altertable_alter_not_null_start(Hypertable *ht, AlterTableCmd *cmd)
process_altertable_alter_not_null(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++)
if (cmd->subtype == AT_DropNotNull)
{
Dimension *dim = &ht->space->dimensions[i];
for (int i = 0; i < ht->space->num_dimensions; i++)
{
Dimension *dim = &ht->space->dimensions[i];

if (IS_OPEN_DIMENSION(dim) &&
strncmp(NameStr(dim->fd.column_name), cmd->name, NAMEDATALEN) == 0)
ereport(ERROR,
(errcode(ERRCODE_TS_OPERATION_NOT_SUPPORTED),
errmsg("cannot drop not-null constraint from a time-partitioned column")));
if (IS_OPEN_DIMENSION(dim) &&
strncmp(NameStr(dim->fd.column_name), cmd->name, NAMEDATALEN) == 0)
ereport(ERROR,
(errcode(ERRCODE_TS_OPERATION_NOT_SUPPORTED),
errmsg("cannot drop not-null constraint from a time-partitioned column")));
}
}
}

Expand Down Expand Up @@ -3849,8 +3894,8 @@ process_altertable_start_table(ProcessUtilityArgs *args)
break;
case AT_SetNotNull:
case AT_DropNotNull:
if (ht != NULL)
process_altertable_alter_not_null_start(ht, cmd);
if (ht)
process_altertable_alter_not_null(ht, cmd);
break;
case AT_AddColumn:
#if PG16_LT
Expand Down
28 changes: 28 additions & 0 deletions tsl/src/compression/compression.c
Original file line number Diff line number Diff line change
Expand Up @@ -1820,6 +1820,34 @@ tsl_compressed_data_info(PG_FUNCTION_ARGS)
return HeapTupleGetDatum(tuple);
}

extern Datum
tsl_compressed_data_has_nulls(PG_FUNCTION_ARGS)
{
const CompressedDataHeader *header = get_compressed_data_header(PG_GETARG_DATUM(0));
bool has_nulls = false;

switch (header->compression_algorithm)
{
case COMPRESSION_ALGORITHM_GORILLA:
has_nulls = gorilla_compressed_has_nulls(header);
break;
case COMPRESSION_ALGORITHM_DICTIONARY:
has_nulls = dictionary_compressed_has_nulls(header);
break;

Check warning on line 1836 in tsl/src/compression/compression.c

View check run for this annotation

Codecov / codecov/patch

tsl/src/compression/compression.c#L1834-L1836

Added lines #L1834 - L1836 were not covered by tests
case COMPRESSION_ALGORITHM_DELTADELTA:
has_nulls = deltadelta_compressed_has_nulls(header);
break;
case COMPRESSION_ALGORITHM_ARRAY:
has_nulls = array_compressed_has_nulls(header);
break;
default:

Check warning on line 1843 in tsl/src/compression/compression.c

View check run for this annotation

Codecov / codecov/patch

tsl/src/compression/compression.c#L1840-L1843

Added lines #L1840 - L1843 were not covered by tests
elog(ERROR, "unknown compression algorithm %d", header->compression_algorithm);
break;
}

return BoolGetDatum(has_nulls);
}

extern CompressionStorage
compression_get_toast_storage(CompressionAlgorithm algorithm)
{
Expand Down
1 change: 1 addition & 0 deletions tsl/src/compression/compression.h
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ extern Datum tsl_compressed_data_recv(PG_FUNCTION_ARGS);
extern Datum tsl_compressed_data_in(PG_FUNCTION_ARGS);
extern Datum tsl_compressed_data_out(PG_FUNCTION_ARGS);
extern Datum tsl_compressed_data_info(PG_FUNCTION_ARGS);
extern Datum tsl_compressed_data_has_nulls(PG_FUNCTION_ARGS);

static void
pg_attribute_unused() assert_num_compression_algorithms_sane(void)
Expand Down
1 change: 1 addition & 0 deletions tsl/src/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ CrossModuleFunctions tsl_cm_functions = {
.compressed_data_in = tsl_compressed_data_in,
.compressed_data_out = tsl_compressed_data_out,
.compressed_data_info = tsl_compressed_data_info,
.compressed_data_has_nulls = tsl_compressed_data_has_nulls,
.deltadelta_compressor_append = tsl_deltadelta_compressor_append,
.deltadelta_compressor_finish = tsl_deltadelta_compressor_finish,
.gorilla_compressor_append = tsl_gorilla_compressor_append,
Expand Down
103 changes: 101 additions & 2 deletions tsl/test/expected/compression_ddl.out
Original file line number Diff line number Diff line change
Expand Up @@ -2543,7 +2543,7 @@ SELECT count(*) FROM test2 WHERE i IS NULL;

\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
ERROR: column "i" of relation "_hyper_44_181_chunk" contains null values
DELETE FROM test2 WHERE i IS NULL;
SELECT count(*) FROM test2 WHERE i IS NULL;
count
Expand All @@ -2552,5 +2552,104 @@ SELECT count(*) FROM test2 WHERE i IS NULL;
(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
-- test set not null
CREATE TABLE test_notnull(time timestamptz, device text, value float);
SELECT create_hypertable('test_notnull','time');
create_hypertable
----------------------------
(46,public,test_notnull,t)
(1 row)

ALTER TABLE test_notnull SET (timescaledb.compress, timescaledb.compress_segmentby='device');
INSERT INTO test_notnull VALUES ('2025-01-01','d1',NULL);
INSERT INTO test_notnull VALUES ('2025-01-01',NULL,NULL);
-- should fail since we have NULL value
\set ON_ERROR_STOP 0
ALTER TABLE test_notnull ALTER COLUMN value SET NOT NULL;
ERROR: column "value" of relation "_hyper_46_238_chunk" contains null values
ALTER TABLE test_notnull ALTER COLUMN device SET NOT NULL;
ERROR: column "device" of relation "_hyper_46_238_chunk" contains null values
\set ON_ERROR_STOP 1
SELECT compress_chunk(show_chunks('test_notnull'));
compress_chunk
-------------------------------------------
_timescaledb_internal._hyper_46_238_chunk
(1 row)

-- should fail since we have NULL value
\set ON_ERROR_STOP 0
ALTER TABLE test_notnull ALTER COLUMN value SET NOT NULL;
ERROR: column "value" of relation "_hyper_46_238_chunk" contains null values
ALTER TABLE test_notnull ALTER COLUMN device SET NOT NULL;
ERROR: column "device" of relation "_hyper_46_238_chunk" contains null values
\set ON_ERROR_STOP 1
UPDATE test_notnull SET value = 1;
ALTER TABLE test_notnull ALTER COLUMN value SET NOT NULL;
ALTER TABLE test_notnull ALTER COLUMN value DROP NOT NULL;
SELECT compress_chunk(show_chunks('test_notnull'));
compress_chunk
-------------------------------------------
_timescaledb_internal._hyper_46_238_chunk
(1 row)

ALTER TABLE test_notnull ALTER COLUMN value SET NOT NULL;
ALTER TABLE test_notnull ALTER COLUMN value DROP NOT NULL;
-- device still has NULL
\set ON_ERROR_STOP 0
ALTER TABLE test_notnull ALTER COLUMN device SET NOT NULL;
ERROR: column "device" of relation "_hyper_46_238_chunk" contains null values
\set ON_ERROR_STOP 1
UPDATE test_notnull SET device = 'd1';
ALTER TABLE test_notnull ALTER COLUMN device SET NOT NULL;
ALTER TABLE test_notnull ALTER COLUMN device DROP NOT NULL;
SELECT compress_chunk(show_chunks('test_notnull'));
compress_chunk
-------------------------------------------
_timescaledb_internal._hyper_46_238_chunk
(1 row)

ALTER TABLE test_notnull ALTER COLUMN device SET NOT NULL;
ALTER TABLE test_notnull ALTER COLUMN device DROP NOT NULL;
-- test partial compressed chunks
-- NULL in uncompressed part only
INSERT INTO test_notnull VALUES ('2025-01-01',NULL,NULL);
\set ON_ERROR_STOP 0
ALTER TABLE test_notnull ALTER COLUMN device SET NOT NULL;
ERROR: column "device" of relation "_hyper_46_238_chunk" contains null values
\set ON_ERROR_STOP 1
-- NULL in compressed part only
SELECT compress_chunk(show_chunks('test_notnull'));
compress_chunk
-------------------------------------------
_timescaledb_internal._hyper_46_238_chunk
(1 row)

INSERT INTO test_notnull VALUES ('2025-01-01','d1',2);
\set ON_ERROR_STOP 0
ALTER TABLE test_notnull ALTER COLUMN device SET NOT NULL;
ERROR: column "device" of relation "_hyper_46_238_chunk" contains null values
\set ON_ERROR_STOP 1
-- test added columns and defaults
ALTER TABLE test_notnull ADD COLUMN c1 int;
\set ON_ERROR_STOP 0
ALTER TABLE test_notnull ALTER COLUMN c1 SET NOT NULL;
ERROR: column "c1" of relation "_hyper_46_238_chunk" contains null values
\set ON_ERROR_STOP 1
ALTER TABLE test_notnull ADD COLUMN c2 int DEFAULT 42;
ALTER TABLE test_notnull ALTER COLUMN c2 SET NOT NULL;
ALTER TABLE test_notnull ALTER COLUMN c2 DROP NOT NULL;
-- test column with default and explicit NULL
UPDATE test_notnull SET c2 = NULL;
\set ON_ERROR_STOP 0
ALTER TABLE test_notnull ALTER COLUMN c2 SET NOT NULL;
ERROR: column "c2" of relation "_hyper_46_238_chunk" contains null values
\set ON_ERROR_STOP 1
SELECT compress_chunk(show_chunks('test_notnull'));
compress_chunk
-------------------------------------------
_timescaledb_internal._hyper_46_238_chunk
(1 row)

-- broken atm due to bug in default handling in compression
ALTER TABLE test_notnull ALTER COLUMN c2 SET NOT NULL;
1 change: 1 addition & 0 deletions tsl/test/shared/expected/extension.out
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ ORDER BY pronamespace::regnamespace::text COLLATE "C", p.oid::regprocedure::text
_timescaledb_functions.chunk_status(regclass)
_timescaledb_functions.chunks_local_size(name,name)
_timescaledb_functions.compressed_chunk_local_stats(name,name)
_timescaledb_functions.compressed_data_has_nulls(_timescaledb_internal.compressed_data)
_timescaledb_functions.compressed_data_in(cstring)
_timescaledb_functions.compressed_data_info(_timescaledb_internal.compressed_data)
_timescaledb_functions.compressed_data_out(_timescaledb_internal.compressed_data)
Expand Down
Loading

0 comments on commit f7bc2fe

Please sign in to comment.