Improve infering start and stop arguments from gapfill query #2059

svenklemm · 2020-07-04T12:46:55Z

Infering start and stop parameter used to only work for top
level constraints. However even when constraints are at the
toplevel in the query they might not end up at the top-level
of the jointree depending on the plan shape. This patch
changes the gapfill code to traverse the jointree to find
valid start and stop arguments.

codecov · 2020-07-04T13:33:05Z

Codecov Report

Merging #2059 into master will increase coverage by 0.12%.
The diff coverage is 92.30%.

@@            Coverage Diff             @@
##           master    #2059      +/-   ##
==========================================
+ Coverage   90.10%   90.23%   +0.12%     
==========================================
  Files         211      211              
  Lines       33596    33580      -16     
==========================================
+ Hits        30273    30300      +27     
+ Misses       3323     3280      -43

Impacted Files	Coverage Δ
tsl/src/nodes/gapfill/exec.c	`96.25% <92.30%> (-0.39%)`	⬇️
src/loader/bgw_launcher.c	`89.53% <0.00%> (-2.47%)`	⬇️
src/import/planner.c	`70.30% <0.00%> (+11.12%)`	⬆️
tsl/src/fdw/shippable.c	`94.28% <0.00%> (+11.42%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4aa8cf...9b45c02. Read the comment docs.

erimatnor

Approving, but there are some nits I think you should fix. There's also an odd error message that I cannot make sense of.

erimatnor · 2020-07-09T08:47:07Z

tsl/src/nodes/gapfill/exec.c

@@ -374,6 +374,99 @@ get_boundary_expr_value(GapFillState *state, GapFillBoundary boundary, Expr *exp
 	return gapfill_datum_get_internal(arg_value, state->gapfill_typid);
 }

+typedef struct collect_boundary_context


Nit: our code style says types/structs should use camelcase like PostgreSQL, i.e., CollectBoundaryContext

erimatnor · 2020-07-09T08:48:12Z

tsl/src/nodes/gapfill/exec.c

+		return false;
+
+	if (IsA(node, FromExpr))
+	{


Nit: elide braces, like above.

tsl/src/nodes/gapfill/exec.c

erimatnor · 2020-07-09T08:53:37Z

tsl/test/expected/gapfill.out

+  time_bucket_gapfill(1,m1.time)
+FROM metrics_int m1 LEFT OUTER JOIN metrics_int m2 ON m1.time=m2.time AND m1.time >=0 AND m1.time < 2
+GROUP BY 1;
+ERROR:  invalid time_bucket_gapfill argument: could not infer start boundary from WHERE clause


This is an odd error message and it doesn't follow the error style guide. Besides, I cannot figure out what it means. Why is it saying invalid gapfill argument and then complaining about the WHERE clause, which neither is an argument to gapfill, nor exists in the query?

Maybe it should just say: errmsg: could not infer start boundary for gapfill. errhint: A gapfill query requires a start boundary on "time" in the WHERE clause.

A gapfill query requires a start and stop arguments, in the first version you had to supply those as arguments, we later added code to infer start and stop from the WHERE-clause however it can only be infered when additional constraints are satisifed (expression needs to be stable, must not reference stuff that cannot be resolved at executor startup). So the error means we couldnt infer start from the WHERE clause and it needs to be supplied as argument to the time_bucket_gapfill call (or WHERE clause needs to be rewritten so it can be infered)

tsl/src/nodes/gapfill/exec.c

pmwkaa · 2020-07-09T13:27:58Z

tsl/src/nodes/gapfill/exec.c

+{
+	List *quals;
+	Var *ts_var;
+} collect_boundary_context;


nit: I guess you dont' have to keep the same name for type and struct, probably better to omit struct name in that case completely.

Infering start and stop parameter used to only work for top level constraints. However even when constraints are at the toplevel in the query they might not end up at the top-level of the jointree depending on the plan shape. This patch changes the gapfill code to traverse the jointree to find valid start and stop arguments.

@akamensky

This maintenance release contains bugfixes since the 1.7.2 release. We deem it high priority for upgrading. In particular the fixes contained in this maintenance release address bugs in compression, drop_chunks and the background worker scheduler. **Bugfixes** * timescale#2059 Improve infering start and stop arguments from gapfill query * timescale#2067 Support moving compressed chunks * timescale#2068 Apply SET TABLESPACE for compressed chunks * timescale#2090 Fix index creation with IF NOT EXISTS for existing indexes * timescale#2092 Fix delete on tables involving hypertables with compression * timescale#2116 Propagate privileges from hypertables to chunks * timescale#2150 Lock dimension slice tuple when scanning * timescale#2164 Fix telemetry installed_time format * timescale#2184 Fix background worker scheduler memory consumption * timescale#2222 Fix negative bitmapset member not allowed in decompression **Thanks** * @akamensky for reporting an issue with drop_chunks * @dewetburger430 for reporting an issue with setting tablespace for compressed chunks * @PichetGoulu for reporting an issue with index creation and IF NOT EXISTS * @sezaru for reporting an issue with background worker scheduler memory consumption

@akamensky

This maintenance release contains bugfixes since the 1.7.2 release. We deem it high priority for upgrading. In particular the fixes contained in this maintenance release address bugs in compression, drop_chunks and the background worker scheduler. **Bugfixes** * timescale#2059 Improve infering start and stop arguments from gapfill query * timescale#2067 Support moving compressed chunks * timescale#2068 Apply SET TABLESPACE for compressed chunks * timescale#2090 Fix index creation with IF NOT EXISTS for existing indexes * timescale#2092 Fix delete on tables involving hypertables with compression * timescale#2116 Propagate privileges from hypertables to chunks * timescale#2150 Lock dimension slice tuple when scanning * timescale#2164 Fix telemetry installed_time format * timescale#2184 Fix background worker scheduler memory consumption * timescale#2222 Fix `negative bitmapset member not allowed` in decompression * timescale#2256 Fix segfault in chunk_append with space partitioning **Thanks** * @akamensky for reporting an issue with drop_chunks and ChunkAppend with space partitioning * @dewetburger430 for reporting an issue with setting tablespace for compressed chunks * @nexces for reporting an issue with ChunkAppend on space-partitioned hypertables * @PichetGoulu for reporting an issue with index creation and IF NOT EXISTS * @sezaru for reporting an issue with background worker scheduler memory consumption

@akamensky

This maintenance release contains bugfixes since the 1.7.2 release. We deem it high priority for upgrading. In particular the fixes contained in this maintenance release address bugs in compression, drop_chunks and the background worker scheduler. **Bugfixes** * timescale#2059 Improve infering start and stop arguments from gapfill query * timescale#2067 Support moving compressed chunks * timescale#2068 Apply SET TABLESPACE for compressed chunks * timescale#2090 Fix index creation with IF NOT EXISTS for existing indexes * timescale#2092 Fix delete on tables involving hypertables with compression * timescale#2116 Propagate privileges from hypertables to chunks * timescale#2150 Lock dimension slice tuple when scanning * timescale#2164 Fix telemetry installed_time format * timescale#2184 Fix background worker scheduler memory consumption * timescale#2222 Fix `negative bitmapset member not allowed` in decompression * timescale#2256 Fix segfault in chunk_append with space partitioning **Thanks** * @akamensky for reporting an issue with drop_chunks and ChunkAppend with space partitioning * @dewetburger430 for reporting an issue with setting tablespace for compressed chunks * @nexces for reporting an issue with ChunkAppend on space-partitioned hypertables * @PichetGoulu for reporting an issue with index creation and IF NOT EXISTS * @sezaru for reporting an issue with background worker scheduler memory consumption

@akamensky

This maintenance release contains bugfixes since the 1.7.2 release. We deem it high priority for upgrading. In particular the fixes contained in this maintenance release address bugs in compression, drop_chunks and the background worker scheduler. **Bugfixes** * timescale#2059 Improve infering start and stop arguments from gapfill query * timescale#2067 Support moving compressed chunks * timescale#2068 Apply SET TABLESPACE for compressed chunks * timescale#2090 Fix index creation with IF NOT EXISTS for existing indexes * timescale#2092 Fix delete on tables involving hypertables with compression * timescale#2116 Propagate privileges from hypertables to chunks * timescale#2150 Lock dimension slice tuple when scanning * timescale#2164 Fix telemetry installed_time format * timescale#2184 Fix background worker scheduler memory consumption * timescale#2222 Fix `negative bitmapset member not allowed` in decompression * timescale#2256 Fix segfault in chunk_append with space partitioning **Thanks** * @akamensky for reporting an issue with drop_chunks and ChunkAppend with space partitioning * @dewetburger430 for reporting an issue with setting tablespace for compressed chunks * @nexces for reporting an issue with ChunkAppend on space-partitioned hypertables * @PichetGoulu for reporting an issue with index creation and IF NOT EXISTS * @sezaru for reporting an issue with background worker scheduler memory consumption

@akamensky

This maintenance release contains bugfixes since the 1.7.2 release. We deem it high priority for upgrading. In particular the fixes contained in this maintenance release address bugs in compression, drop_chunks and the background worker scheduler. **Bugfixes** * timescale#2059 Improve infering start and stop arguments from gapfill query * timescale#2067 Support moving compressed chunks * timescale#2068 Apply SET TABLESPACE for compressed chunks * timescale#2090 Fix index creation with IF NOT EXISTS for existing indexes * timescale#2092 Fix delete on tables involving hypertables with compression * timescale#2164 Fix telemetry installed_time format * timescale#2184 Fix background worker scheduler memory consumption * timescale#2222 Fix `negative bitmapset member not allowed` in decompression * timescale#2255 Propagate privileges from hypertables to chunks * timescale#2256 Fix segfault in chunk_append with space partitioning * timescale#2259 Fix recursion in cache processing * timescale#2261 Lock dimension slice tuple when scanning **Thanks** * @akamensky for reporting an issue with drop_chunks and ChunkAppend with space partitioning * @dewetburger430 for reporting an issue with setting tablespace for compressed chunks * @fvannee for reporting an issue with cache invalidation * @nexces for reporting an issue with ChunkAppend on space-partitioned hypertables * @PichetGoulu for reporting an issue with index creation and IF NOT EXISTS * @sezaru for reporting an issue with background worker scheduler memory consumption

@akamensky

This maintenance release contains bugfixes since the 1.7.2 release. We deem it high priority for upgrading. In particular the fixes contained in this maintenance release address bugs in compression, drop_chunks and the background worker scheduler. **Bugfixes** * timescale#2059 Improve infering start and stop arguments from gapfill query * timescale#2067 Support moving compressed chunks * timescale#2068 Apply SET TABLESPACE for compressed chunks * timescale#2090 Fix index creation with IF NOT EXISTS for existing indexes * timescale#2092 Fix delete on tables involving hypertables with compression * timescale#2164 Fix telemetry installed_time format * timescale#2184 Fix background worker scheduler memory consumption * timescale#2222 Fix `negative bitmapset member not allowed` in decompression * timescale#2255 Propagate privileges from hypertables to chunks * timescale#2256 Fix segfault in chunk_append with space partitioning * timescale#2259 Fix recursion in cache processing * timescale#2261 Lock dimension slice tuple when scanning **Thanks** * @akamensky for reporting an issue with drop_chunks and ChunkAppend with space partitioning * @dewetburger430 for reporting an issue with setting tablespace for compressed chunks * @fvannee for reporting an issue with cache invalidation * @nexces for reporting an issue with ChunkAppend on space-partitioned hypertables * @PichetGoulu for reporting an issue with index creation and IF NOT EXISTS * @sezaru for reporting an issue with background worker scheduler memory consumption

@akamensky

This maintenance release contains bugfixes since the 1.7.2 release. We deem it high priority for upgrading. In particular the fixes contained in this maintenance release address issues in compression, drop_chunks and the background worker scheduler. **Bugfixes** * timescale#2059 Improve infering start and stop arguments from gapfill query * timescale#2067 Support moving compressed chunks * timescale#2068 Apply SET TABLESPACE for compressed chunks * timescale#2090 Fix index creation with IF NOT EXISTS for existing indexes * timescale#2092 Fix delete on tables involving hypertables with compression * timescale#2164 Fix telemetry installed_time format * timescale#2184 Fix background worker scheduler memory consumption * timescale#2222 Fix `negative bitmapset member not allowed` in decompression * timescale#2255 Propagate privileges from hypertables to chunks * timescale#2256 Fix segfault in chunk_append with space partitioning * timescale#2259 Fix recursion in cache processing * timescale#2261 Lock dimension slice tuple when scanning **Thanks** * @akamensky for reporting an issue with drop_chunks and ChunkAppend with space partitioning * @dewetburger430 for reporting an issue with setting tablespace for compressed chunks * @fvannee for reporting an issue with cache invalidation * @nexces for reporting an issue with ChunkAppend on space-partitioned hypertables * @PichetGoulu for reporting an issue with index creation and IF NOT EXISTS * @prathamesh-sonpatki for contributing a typo fix * @sezaru for reporting an issue with background worker scheduler memory consumption

@akamensky

This maintenance release contains bugfixes since the 1.7.2 release. We deem it high priority for upgrading. In particular the fixes contained in this maintenance release address issues in compression, drop_chunks and the background worker scheduler. **Bugfixes** * #2059 Improve infering start and stop arguments from gapfill query * #2067 Support moving compressed chunks * #2068 Apply SET TABLESPACE for compressed chunks * #2090 Fix index creation with IF NOT EXISTS for existing indexes * #2092 Fix delete on tables involving hypertables with compression * #2164 Fix telemetry installed_time format * #2184 Fix background worker scheduler memory consumption * #2222 Fix `negative bitmapset member not allowed` in decompression * #2255 Propagate privileges from hypertables to chunks * #2256 Fix segfault in chunk_append with space partitioning * #2259 Fix recursion in cache processing * #2261 Lock dimension slice tuple when scanning **Thanks** * @akamensky for reporting an issue with drop_chunks and ChunkAppend with space partitioning * @dewetburger430 for reporting an issue with setting tablespace for compressed chunks * @fvannee for reporting an issue with cache invalidation * @nexces for reporting an issue with ChunkAppend on space-partitioned hypertables * @PichetGoulu for reporting an issue with index creation and IF NOT EXISTS * @prathamesh-sonpatki for contributing a typo fix * @sezaru for reporting an issue with background worker scheduler memory consumption

@akamensky

This maintenance release contains bugfixes since the 1.7.2 release. We deem it high priority for upgrading. In particular the fixes contained in this maintenance release address issues in compression, drop_chunks and the background worker scheduler. **Bugfixes** * #2059 Improve infering start and stop arguments from gapfill query * #2067 Support moving compressed chunks * #2068 Apply SET TABLESPACE for compressed chunks * #2090 Fix index creation with IF NOT EXISTS for existing indexes * #2092 Fix delete on tables involving hypertables with compression * #2164 Fix telemetry installed_time format * #2184 Fix background worker scheduler memory consumption * #2222 Fix `negative bitmapset member not allowed` in decompression * #2255 Propagate privileges from hypertables to chunks * #2256 Fix segfault in chunk_append with space partitioning * #2259 Fix recursion in cache processing * #2261 Lock dimension slice tuple when scanning **Thanks** * @akamensky for reporting an issue with drop_chunks and ChunkAppend with space partitioning * @dewetburger430 for reporting an issue with setting tablespace for compressed chunks * @fvannee for reporting an issue with cache invalidation * @nexces for reporting an issue with ChunkAppend on space-partitioned hypertables * @PichetGoulu for reporting an issue with index creation and IF NOT EXISTS * @prathamesh-sonpatki for contributing a typo fix * @sezaru for reporting an issue with background worker scheduler memory consumption

svenklemm requested a review from a team as a code owner July 4, 2020 12:46

svenklemm requested review from pmwkaa, mkindahl and gayyappan and removed request for a team July 4, 2020 12:46

svenklemm added the gapfill label Jul 4, 2020

svenklemm force-pushed the gapfill_boundaries branch 2 times, most recently from 1ebabae to b5ce99d Compare July 4, 2020 19:45

svenklemm requested review from erimatnor, k-rus and WireBaron July 6, 2020 05:22

svenklemm force-pushed the gapfill_boundaries branch 2 times, most recently from 6a79dc8 to c850fc6 Compare July 8, 2020 12:46

erimatnor approved these changes Jul 9, 2020

View reviewed changes

pmwkaa approved these changes Jul 9, 2020

View reviewed changes

svenklemm force-pushed the gapfill_boundaries branch from c850fc6 to 16050ab Compare July 10, 2020 08:47

svenklemm force-pushed the gapfill_boundaries branch from 16050ab to 9b45c02 Compare July 10, 2020 09:03

svenklemm merged commit 665d56b into timescale:master Jul 10, 2020

svenklemm mentioned this pull request Aug 24, 2020

Release 1.7.3 #2254

Merged

svenklemm deleted the gapfill_boundaries branch April 18, 2021 14:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve infering start and stop arguments from gapfill query #2059

Improve infering start and stop arguments from gapfill query #2059

svenklemm commented Jul 4, 2020

codecov bot commented Jul 4, 2020 •

edited

Loading

erimatnor left a comment

erimatnor Jul 9, 2020

erimatnor Jul 9, 2020

erimatnor Jul 9, 2020

svenklemm Jul 9, 2020 •

edited

Loading

pmwkaa Jul 9, 2020

Improve infering start and stop arguments from gapfill query #2059

Improve infering start and stop arguments from gapfill query #2059

Conversation

svenklemm commented Jul 4, 2020

codecov bot commented Jul 4, 2020 • edited Loading

Codecov Report

erimatnor left a comment

Choose a reason for hiding this comment

erimatnor Jul 9, 2020

Choose a reason for hiding this comment

erimatnor Jul 9, 2020

Choose a reason for hiding this comment

erimatnor Jul 9, 2020

Choose a reason for hiding this comment

svenklemm Jul 9, 2020 • edited Loading

Choose a reason for hiding this comment

pmwkaa Jul 9, 2020

Choose a reason for hiding this comment

codecov bot commented Jul 4, 2020 •

edited

Loading

svenklemm Jul 9, 2020 •

edited

Loading