From 22dcc251c3c1821b229d832eaf534e09fa8969c1 Mon Sep 17 00:00:00 2001 From: Burak Velioglu Date: Fri, 27 Aug 2021 13:24:08 +0300 Subject: [PATCH] Fix tests and adress reviews --- .../timeseries/create_timeseries_table.c | 20 ++++++++++--- .../timeseries/drop_timeseries_table.c | 7 +++-- .../sql/timeseries--10.1-1--10.2-1.sql | 12 ++++---- .../get_missing_partition_ranges/10.2-1.sql | 2 +- .../get_missing_partition_ranges/latest.sql | 2 +- src/backend/timeseries/timeseries_utils.c | 28 +++++++++++-------- src/include/timeseries/timeseries_utils.h | 1 + ...imeseries_create_drop_timeseries_table.out | 8 +++--- .../expected/upgrade_list_citus_objects.out | 2 +- ...imeseries_create_drop_timeseries_table.sql | 8 +++--- ...imeseries_get_missing_partition_ranges.sql | 2 +- 11 files changed, 56 insertions(+), 36 deletions(-) diff --git a/src/backend/timeseries/create_timeseries_table.c b/src/backend/timeseries/create_timeseries_table.c index cac573950..3763c392c 100644 --- a/src/backend/timeseries/create_timeseries_table.c +++ b/src/backend/timeseries/create_timeseries_table.c @@ -192,7 +192,7 @@ ErrorIfNotSuitableToConvertTimeseriesTable(Oid relationId, Interval *partitionIn /* * Create the initial pre and post make partitions for the given relation id - * by getting the related information from citus_timeseries_tables and utilizing + * by getting the related information from timeseries.tables and utilizing * create_missing_partitions */ static void @@ -209,13 +209,25 @@ InitiateTimeseriesTablePartitions(Oid relationId, bool useStartFrom) if (useStartFrom) { appendStringInfo(initiateTimeseriesPartitionsCommand, - "SELECT create_missing_partitions(logicalrelid, now() + partitioninterval * postmakeintervalcount, startfrom) from citus_timeseries.citus_timeseries_tables WHERE logicalrelid = %d;", + "SELECT " + "pg_catalog.create_missing_partitions(" + "logicalrelid," + "now() + partitioninterval * postmakeintervalcount," + "startfrom) " + "FROM timeseries.tables " + "WHERE logicalrelid = %d;", relationId); } else { appendStringInfo(initiateTimeseriesPartitionsCommand, - "SELECT create_missing_partitions(logicalrelid, now() + partitioninterval * postmakeintervalcount, now() - partitioninterval * premakeintervalcount) from citus_timeseries.citus_timeseries_tables WHERE logicalrelid = %d;", + "SELECT " + "pg_catalog.create_missing_partitions(" + "logicalrelid," + "now() + partitioninterval * postmakeintervalcount," + "now() - partitioninterval * premakeintervalcount) " + "FROM timeseries.tables " + "WHERE logicalrelid = %d;", relationId); } @@ -233,7 +245,7 @@ InitiateTimeseriesTablePartitions(Oid relationId, bool useStartFrom) /* - * Add tuples for the given table to the citus_timeseries_tables using given params + * Add tuples for the given table to the timeseries.tables using given params */ static void InsertIntoCitusTimeseriesTables(Oid relationId, Interval *partitionInterval, diff --git a/src/backend/timeseries/drop_timeseries_table.c b/src/backend/timeseries/drop_timeseries_table.c index 1976a100a..4f5e17227 100644 --- a/src/backend/timeseries/drop_timeseries_table.c +++ b/src/backend/timeseries/drop_timeseries_table.c @@ -46,15 +46,16 @@ drop_timeseries_table(PG_FUNCTION_ARGS) ScanKeyData relIdKey[1]; Relation timeseriesRelation = table_open(CitusTimeseriesTablesRelationId(), - AccessShareLock); + RowExclusiveLock); + Oid pkeyOid = CitusTimeseriesTablesPKeyIndexRelationId(); ScanKeyInit(&relIdKey[0], Anum_citus_timeseries_table_relation_id, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(relationId)); - SysScanDesc timeseriesRelScan = systable_beginscan(timeseriesRelation, InvalidOid, - false, NULL, 1, relIdKey); + SysScanDesc timeseriesRelScan = systable_beginscan(timeseriesRelation, pkeyOid, + true, NULL, 1, relIdKey); HeapTuple timeseriesTuple = systable_getnext(timeseriesRelScan); if (HeapTupleIsValid(timeseriesTuple)) diff --git a/src/backend/timeseries/sql/timeseries--10.1-1--10.2-1.sql b/src/backend/timeseries/sql/timeseries--10.1-1--10.2-1.sql index cc8375729..d2d5e1ea7 100644 --- a/src/backend/timeseries/sql/timeseries--10.1-1--10.2-1.sql +++ b/src/backend/timeseries/sql/timeseries--10.1-1--10.2-1.sql @@ -1,9 +1,9 @@ /* timeseries--10.1-1--10.2-1.sql */ -CREATE SCHEMA citus_timeseries; -SET search_path TO citus_timeseries; +CREATE SCHEMA timeseries; +SET search_path TO timeseries; -CREATE TABLE citus_timeseries_tables ( +CREATE TABLE tables ( logicalrelid regclass NOT NULL PRIMARY KEY, partitioninterval INTERVAL NOT NULL, postmakeintervalcount INT NOT NULL, @@ -13,10 +13,10 @@ CREATE TABLE citus_timeseries_tables ( retentionthreshold INTERVAL ); -COMMENT ON TABLE citus_timeseries_tables IS 'Keeps interval and threshold informations for timeseries tables'; +COMMENT ON TABLE tables IS 'Keeps interval and threshold informations for timeseries tables'; -- grant read access for timeseries metadata tables to unprivileged user -GRANT USAGE ON SCHEMA citus_timeseries TO PUBLIC; -GRANT SELECT ON ALL tables IN SCHEMA citus_timeseries TO PUBLIC; +GRANT USAGE ON SCHEMA timeseries TO PUBLIC; +GRANT SELECT ON ALL tables IN SCHEMA timeseries TO PUBLIC; RESET search_path; diff --git a/src/backend/timeseries/sql/udfs/get_missing_partition_ranges/10.2-1.sql b/src/backend/timeseries/sql/udfs/get_missing_partition_ranges/10.2-1.sql index 7bd63adf2..a543967fd 100644 --- a/src/backend/timeseries/sql/udfs/get_missing_partition_ranges/10.2-1.sql +++ b/src/backend/timeseries/sql/udfs/get_missing_partition_ranges/10.2-1.sql @@ -24,7 +24,7 @@ BEGIN SELECT partitioninterval INTO table_partition_interval - FROM citus_timeseries.citus_timeseries_tables + FROM timeseries.tables WHERE logicalrelid = table_name; IF NOT found THEN diff --git a/src/backend/timeseries/sql/udfs/get_missing_partition_ranges/latest.sql b/src/backend/timeseries/sql/udfs/get_missing_partition_ranges/latest.sql index 7bd63adf2..a543967fd 100644 --- a/src/backend/timeseries/sql/udfs/get_missing_partition_ranges/latest.sql +++ b/src/backend/timeseries/sql/udfs/get_missing_partition_ranges/latest.sql @@ -24,7 +24,7 @@ BEGIN SELECT partitioninterval INTO table_partition_interval - FROM citus_timeseries.citus_timeseries_tables + FROM timeseries.tables WHERE logicalrelid = table_name; IF NOT found THEN diff --git a/src/backend/timeseries/timeseries_utils.c b/src/backend/timeseries/timeseries_utils.c index caa60319e..d624b1492 100644 --- a/src/backend/timeseries/timeseries_utils.c +++ b/src/backend/timeseries/timeseries_utils.c @@ -23,16 +23,16 @@ #include "timeseries/timeseries_utils.h" /* - * Get the relation id for citus_timeseries_tables metadata table + * Get the relation id for timeseries.tables metadata table */ Oid CitusTimeseriesTablesRelationId() { - Oid relationId = get_relname_relid("citus_timeseries_tables", + Oid relationId = get_relname_relid("tables", TimeseriesNamespaceId()); if (relationId == InvalidOid) { - ereport(ERROR, (errmsg("cache lookup failed for citus_timeseries_tables," + ereport(ERROR, (errmsg("cache lookup failed for timeseries tables," "called too early?"))); } @@ -40,6 +40,16 @@ CitusTimeseriesTablesRelationId() } +/* + * CitusTimeseriesTablesPKeyIndexRelationId returns relation id of timeseries.tables_pkey. + */ +Oid +CitusTimeseriesTablesPKeyIndexRelationId() +{ + return get_relname_relid("tables_pkey", TimeseriesNamespaceId()); +} + + /* * TimeseriesNamespaceId returns namespace id of the schema we store timeseries * related metadata tables. @@ -47,7 +57,7 @@ CitusTimeseriesTablesRelationId() Oid TimeseriesNamespaceId() { - return get_namespace_oid("citus_timeseries", false); + return get_namespace_oid("timeseries", false); } @@ -83,13 +93,9 @@ bool CheckIntervalAlignnmentWithPartitionKey(PartitionKey partitionKey, Interval *partitionInterval) { - Oid partTypeId; - HeapTuple typeTuple; - Form_pg_type typeForm; - - partTypeId = partitionKey->parttypid[0]; - typeTuple = SearchSysCache1(TYPEOID, partTypeId); - typeForm = (Form_pg_type) GETSTRUCT(typeTuple); + Oid partTypeId = partitionKey->parttypid[0]; + HeapTuple typeTuple = SearchSysCache1(TYPEOID, partTypeId); + Form_pg_type typeForm = (Form_pg_type) GETSTRUCT(typeTuple); ReleaseSysCache(typeTuple); if (strncmp(typeForm->typname.data, "date", NAMEDATALEN) == 0) diff --git a/src/include/timeseries/timeseries_utils.h b/src/include/timeseries/timeseries_utils.h index f1dc1f736..a40b47a5e 100644 --- a/src/include/timeseries/timeseries_utils.h +++ b/src/include/timeseries/timeseries_utils.h @@ -33,6 +33,7 @@ (ivp)->month * (30.0 * SECS_PER_DAY)) extern Oid CitusTimeseriesTablesRelationId(void); +extern Oid CitusTimeseriesTablesPKeyIndexRelationId(void); extern Oid TimeseriesNamespaceId(void); extern bool CheckIntervalAlignmentWithThresholds(Interval *partitionInterval, Interval *compressionThreshold, diff --git a/src/test/regress/expected/timeseries_create_drop_timeseries_table.out b/src/test/regress/expected/timeseries_create_drop_timeseries_table.out index 8ee91048a..0d2e9c791 100644 --- a/src/test/regress/expected/timeseries_create_drop_timeseries_table.out +++ b/src/test/regress/expected/timeseries_create_drop_timeseries_table.out @@ -380,14 +380,14 @@ SELECT create_timeseries_table('drop_check_test_partitioned_table', INTERVAL '2 (1 row) -SELECT * FROM citus_timeseries.citus_timeseries_tables; +SELECT * FROM timeseries.tables; logicalrelid | partitioninterval | postmakeintervalcount | premakeintervalcount | startfrom | compressionthreshold | retentionthreshold --------------------------------------------------------------------- drop_check_test_partitioned_table | @ 2 hours | 7 | 7 | | | (1 row) DROP TABLE drop_check_test_partitioned_table; -SELECT * FROM citus_timeseries.citus_timeseries_tables; +SELECT * FROM timeseries.tables; logicalrelid | partitioninterval | postmakeintervalcount | premakeintervalcount | startfrom | compressionthreshold | retentionthreshold --------------------------------------------------------------------- (0 rows) @@ -403,14 +403,14 @@ SELECT create_timeseries_table('drop_check_test_partitioned_table', INTERVAL '2 (1 row) -SELECT * FROM citus_timeseries.citus_timeseries_tables; +SELECT * FROM timeseries.tables; logicalrelid | partitioninterval | postmakeintervalcount | premakeintervalcount | startfrom | compressionthreshold | retentionthreshold --------------------------------------------------------------------- drop_check_test_partitioned_table | @ 2 hours | 7 | 7 | | | (1 row) DROP TABLE drop_check_test_partitioned_table; -SELECT * FROM citus_timeseries.citus_timeseries_tables; +SELECT * FROM timeseries.tables; logicalrelid | partitioninterval | postmakeintervalcount | premakeintervalcount | startfrom | compressionthreshold | retentionthreshold --------------------------------------------------------------------- (0 rows) diff --git a/src/test/regress/expected/upgrade_list_citus_objects.out b/src/test/regress/expected/upgrade_list_citus_objects.out index 627690ef0..6bc173450 100644 --- a/src/test/regress/expected/upgrade_list_citus_objects.out +++ b/src/test/regress/expected/upgrade_list_citus_objects.out @@ -230,7 +230,7 @@ ORDER BY 1; sequence pg_dist_placement_placementid_seq sequence pg_dist_shardid_seq table citus.pg_dist_object - table citus_timeseries.citus_timeseries_tables + table timeseries.tables table columnar.chunk table columnar.chunk_group table columnar.options diff --git a/src/test/regress/sql/timeseries_create_drop_timeseries_table.sql b/src/test/regress/sql/timeseries_create_drop_timeseries_table.sql index d06f5b886..1f5667442 100644 --- a/src/test/regress/sql/timeseries_create_drop_timeseries_table.sql +++ b/src/test/regress/sql/timeseries_create_drop_timeseries_table.sql @@ -224,9 +224,9 @@ CREATE TABLE drop_check_test_partitioned_table( measure_data integer) PARTITION BY RANGE(eventdatetime); SELECT create_timeseries_table('drop_check_test_partitioned_table', INTERVAL '2 hours'); -SELECT * FROM citus_timeseries.citus_timeseries_tables; +SELECT * FROM timeseries.tables; DROP TABLE drop_check_test_partitioned_table; -SELECT * FROM citus_timeseries.citus_timeseries_tables; +SELECT * FROM timeseries.tables; BEGIN; CREATE TABLE drop_check_test_partitioned_table( @@ -234,7 +234,7 @@ BEGIN; eventdatetime timestamp with time zone, measure_data integer) PARTITION BY RANGE(eventdatetime); SELECT create_timeseries_table('drop_check_test_partitioned_table', INTERVAL '2 hours'); - SELECT * FROM citus_timeseries.citus_timeseries_tables; + SELECT * FROM timeseries.tables; DROP TABLE drop_check_test_partitioned_table; - SELECT * FROM citus_timeseries.citus_timeseries_tables; + SELECT * FROM timeseries.tables; COMMIT; diff --git a/src/test/regress/sql/timeseries_get_missing_partition_ranges.sql b/src/test/regress/sql/timeseries_get_missing_partition_ranges.sql index 14084be9f..e339f2441 100644 --- a/src/test/regress/sql/timeseries_get_missing_partition_ranges.sql +++ b/src/test/regress/sql/timeseries_get_missing_partition_ranges.sql @@ -80,7 +80,7 @@ ROLLBACK; BEGIN; SELECT create_timeseries_table('tstz_partitioned_table', INTERVAL '6 hours'); - SELECT + SELECT date_trunc('hour', now()) - range_from_value::timestamp with time zone as from_diff, date_trunc('hour', now()) - range_to_value::timestamp with time zone as to_diff FROM get_missing_partition_ranges('tstz_partitioned_table', now() + INTERVAL '1 day', now() - INTERVAL '1 day')