Address review comments

timescale · Sep 15, 2023 · 0419d25 · 0419d25
1 parent bb61846
commit 0419d25
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 18 deletions.
diff --git a/src/hypertable.c b/src/hypertable.c
@@ -3024,8 +3024,8 @@ ts_hypertable_update_dimension_partitions(const Hypertable *ht)
 /*
  * hypertable_osm_range_update
  * 0 hypertable REGCLASS,
- * 1 range_start,
- * 2 range_end,
+ * 1 range_start=NULL::bigint,
+ * 2 range_end=NULL,
  * 3 empty=false
  * If empty is set to true then the range will be set to invalid range
  * but the overlap flag will be unset, indicating that no data is managed
@@ -3068,10 +3068,13 @@ ts_hypertable_osm_range_update(PG_FUNCTION_ARGS)
 	/*
 	 * range_start, range_end arguments must be converted to internal representation
 	 * a NULL start value is interpreted as INT64_MAX - 1 and a NULL end value is
-	 * interpreted as infinity.
+	 * interpreted as INT64_MAX.
 	 * Passing both start and end NULL values will reset the range to the default range an
 	 * OSM chunk is given upon creation, which is [INT64_MAX - 1, INT64_MAX]
 	 */
+	if ((PG_ARGISNULL(1) && !PG_ARGISNULL(2)) || (!PG_ARGISNULL(1) && PG_ARGISNULL(2)))
+		elog(ERROR, "range_start and range_end parameters must be both NULL or both non-NULL");
+
 	int64 range_start_internal, range_end_internal;
 	if (PG_ARGISNULL(1))
 		range_start_internal = PG_INT64_MAX - 1;
@@ -3117,19 +3120,17 @@ ts_hypertable_osm_range_update(PG_FUNCTION_ARGS)
 				errhint("Range should be set to invalid for tiered chunk"));
 	range_invalid = ts_osm_chunk_range_is_invalid(range_start_internal, range_end_internal);
 	/* Update the hypertable flags regarding the validity of the OSM range */
-	if (overlap || range_invalid)
+	if (range_invalid)
 	{
+		/* range is set to infinity so the OSM chunk is considered last */
+		range_start_internal = PG_INT64_MAX - 1;
+		range_end_internal = PG_INT64_MAX;
 		if (!osm_chunk_empty)
 			ht->fd.status =
 				ts_set_flags_32(ht->fd.status, HYPERTABLE_STATUS_OSM_CHUNK_NONCONTIGUOUS);
 		else
-		{
-			/* range is also set to infinity so the OSM chunk is considered last */
-			range_start_internal = PG_INT64_MAX - 1;
-			range_end_internal = PG_INT64_MAX;
 			ht->fd.status =
 				ts_clear_flags_32(ht->fd.status, HYPERTABLE_STATUS_OSM_CHUNK_NONCONTIGUOUS);
-		}
 	}
 	else
 		ht->fd.status = ts_clear_flags_32(ht->fd.status, HYPERTABLE_STATUS_OSM_CHUNK_NONCONTIGUOUS);

diff --git a/tsl/test/expected/chunk_utils_internal.out b/tsl/test/expected/chunk_utils_internal.out
@@ -1132,8 +1132,17 @@ WHERE c.hypertable_id = :htid AND cc.chunk_id = c.id AND ds.id = cc.dimension_sl
 
 -- check that range was reset to default - infinity
 \set ON_ERROR_STOP 0
+-- both range_start and range_end must be NULL, or non-NULL
 SELECT _timescaledb_functions.hypertable_osm_range_update('test_multicon', NULL, '2020-01-04 01:00'::timestamptz);
-ERROR:  dimension slice range_end cannot be less than range_start
+ERROR:  range_start and range_end parameters must be both NULL or both non-NULL
+SELECT _timescaledb_functions.hypertable_osm_range_update('test_multicon', NULL, NULL);
+ERROR:  could not determine polymorphic type because input has type unknown
+SELECT _timescaledb_functions.hypertable_osm_range_update('test_multicon');
+ hypertable_osm_range_update 
+-----------------------------
+ f
+(1 row)
+
 \set ON_ERROR_STOP 1
 SELECT _timescaledb_functions.hypertable_osm_range_update('test_multicon', NULL::timestamptz, NULL);
  hypertable_osm_range_update 
@@ -1227,17 +1236,15 @@ SELECT ts_setup_osm_hook();
 \set ON_ERROR_STOP 0
 INSERT INTO test_chunkapp VALUES ('2020-01-03 02:00'::timestamptz, 3);
 ERROR:  Cannot insert into tiered chunk range of public.test_chunkapp
--- check that range is reset to default - infinity
-SELECT _timescaledb_functions.hypertable_osm_range_update('test_chunkapp', NULL, '2020-01-04 01:00'::timestamptz);
-ERROR:  dimension slice range_end cannot be less than range_start
 \set ON_ERROR_STOP 1
 SELECT ts_undo_osm_hook();
  ts_undo_osm_hook 
 ------------------
 
 (1 row)
 
-SELECT _timescaledb_functions.hypertable_osm_range_update('test_chunkapp', NULL::timestamptz, NULL);
+-- reset range to infinity
+SELECT _timescaledb_functions.hypertable_osm_range_update('test_chunkapp',empty:=false);
  hypertable_osm_range_update 
 -----------------------------
  f

diff --git a/tsl/test/sql/chunk_utils_internal.sql b/tsl/test/sql/chunk_utils_internal.sql
@@ -645,7 +645,10 @@ FROM _timescaledb_catalog.chunk c, _timescaledb_catalog.chunk_constraint cc, _ti
 WHERE c.hypertable_id = :htid AND cc.chunk_id = c.id AND ds.id = cc.dimension_slice_id;
 -- check that range was reset to default - infinity
 \set ON_ERROR_STOP 0
+-- both range_start and range_end must be NULL, or non-NULL
 SELECT _timescaledb_functions.hypertable_osm_range_update('test_multicon', NULL, '2020-01-04 01:00'::timestamptz);
+SELECT _timescaledb_functions.hypertable_osm_range_update('test_multicon', NULL, NULL);
+SELECT _timescaledb_functions.hypertable_osm_range_update('test_multicon');
 \set ON_ERROR_STOP 1
 SELECT _timescaledb_functions.hypertable_osm_range_update('test_multicon', NULL::timestamptz, NULL);
 SELECT cc.chunk_id, c.table_name, c.status, c.osm_chunk, cc.dimension_slice_id, ds.range_start, ds.range_end
@@ -685,12 +688,10 @@ SELECT * FROM test_chunkapp ORDER BY 1;
 SELECT ts_setup_osm_hook();
 \set ON_ERROR_STOP 0
 INSERT INTO test_chunkapp VALUES ('2020-01-03 02:00'::timestamptz, 3);
--- check that range is reset to default - infinity
-SELECT _timescaledb_functions.hypertable_osm_range_update('test_chunkapp', NULL, '2020-01-04 01:00'::timestamptz);
 \set ON_ERROR_STOP 1
 SELECT ts_undo_osm_hook();
-
-SELECT _timescaledb_functions.hypertable_osm_range_update('test_chunkapp', NULL::timestamptz, NULL);
+-- reset range to infinity
+SELECT _timescaledb_functions.hypertable_osm_range_update('test_chunkapp',empty:=false);
 -- ordered append not possible because range is invalid and empty was not specified
 EXPLAIN SELECT * FROM test_chunkapp ORDER BY 1;
 SELECT * FROM test_chunkapp ORDER BY 1;