diff --git a/.unreleased/pr_7707 b/.unreleased/pr_7707 new file mode 100644 index 00000000000..fe7139260a5 --- /dev/null +++ b/.unreleased/pr_7707 @@ -0,0 +1 @@ +Implements: #7707 Support ALTER COLUMN SET NOT NULL on compressed chunks diff --git a/sql/pre_install/types.functions.sql b/sql/pre_install/types.functions.sql index ced40c02d24..d4f34f38570 100644 --- a/sql/pre_install/types.functions.sql +++ b/sql/pre_install/types.functions.sql @@ -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 diff --git a/sql/updates/latest-dev.sql b/sql/updates/latest-dev.sql index aa1d13abc16..14a5835c085 100644 --- a/sql/updates/latest-dev.sql +++ b/sql/updates/latest-dev.sql @@ -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'; diff --git a/sql/updates/reverse-dev.sql b/sql/updates/reverse-dev.sql index e59e84b01ef..1dec68d6120 100644 --- a/sql/updates/reverse-dev.sql +++ b/sql/updates/reverse-dev.sql @@ -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); + diff --git a/src/cross_module_fn.c b/src/cross_module_fn.c index 4afc583340a..881a929d33b 100644 --- a/src/cross_module_fn.c +++ b/src/cross_module_fn.c @@ -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); diff --git a/src/cross_module_fn.h b/src/cross_module_fn.h index 8db0f9123d6..f6f89eea5d1 100644 --- a/src/cross_module_fn.h +++ b/src/cross_module_fn.h @@ -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); diff --git a/src/process_utility.c b/src/process_utility.c index c932369ec46..6e7220d700f 100644 --- a/src/process_utility.c +++ b/src/process_utility.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -2570,10 +2571,6 @@ 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) @@ -2581,46 +2578,94 @@ 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"))); + } } } @@ -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 diff --git a/tsl/src/compression/compression.c b/tsl/src/compression/compression.c index 94259fce222..e9128e11f28 100644 --- a/tsl/src/compression/compression.c +++ b/tsl/src/compression/compression.c @@ -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; + 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: + elog(ERROR, "unknown compression algorithm %d", header->compression_algorithm); + break; + } + + return BoolGetDatum(has_nulls); +} + extern CompressionStorage compression_get_toast_storage(CompressionAlgorithm algorithm) { diff --git a/tsl/src/compression/compression.h b/tsl/src/compression/compression.h index 25d2c9c5fd1..20dca32c1f2 100644 --- a/tsl/src/compression/compression.h +++ b/tsl/src/compression/compression.h @@ -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) diff --git a/tsl/src/init.c b/tsl/src/init.c index 219ee2cb392..6a049668dd8 100644 --- a/tsl/src/init.c +++ b/tsl/src/init.c @@ -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, diff --git a/tsl/test/expected/compression_ddl.out b/tsl/test/expected/compression_ddl.out index 340ceb11ad6..891ed5e44eb 100644 --- a/tsl/test/expected/compression_ddl.out +++ b/tsl/test/expected/compression_ddl.out @@ -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 @@ -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; diff --git a/tsl/test/shared/expected/extension.out b/tsl/test/shared/expected/extension.out index 310d4ba836c..24ac7500e8f 100644 --- a/tsl/test/shared/expected/extension.out +++ b/tsl/test/shared/expected/extension.out @@ -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) diff --git a/tsl/test/sql/compression_ddl.sql b/tsl/test/sql/compression_ddl.sql index 35ab26d35f6..23b542fc9ea 100644 --- a/tsl/test/sql/compression_ddl.sql +++ b/tsl/test/sql/compression_ddl.sql @@ -1078,3 +1078,77 @@ DELETE FROM test2 WHERE i IS NULL; SELECT count(*) FROM test2 WHERE i IS NULL; ALTER TABLE test2 ALTER COLUMN i SET NOT NULL; \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'); +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; +ALTER TABLE test_notnull ALTER COLUMN device SET NOT NULL; +\set ON_ERROR_STOP 1 + +SELECT compress_chunk(show_chunks('test_notnull')); +-- should fail since we have NULL value +\set ON_ERROR_STOP 0 +ALTER TABLE test_notnull ALTER COLUMN value SET NOT NULL; +ALTER TABLE test_notnull ALTER COLUMN device SET NOT NULL; +\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')); + +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; +\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')); +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; +\set ON_ERROR_STOP 1 + +-- NULL in compressed part only +SELECT compress_chunk(show_chunks('test_notnull')); +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; +\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; +\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; +\set ON_ERROR_STOP 1 +SELECT compress_chunk(show_chunks('test_notnull')); +-- broken atm due to bug in default handling in compression +ALTER TABLE test_notnull ALTER COLUMN c2 SET NOT NULL; +