Skip to content

Comments

Merge v11#11

Open
did-g wants to merge 78 commits intodavidkohn88:masterfrom
did-g:merge_v11
Open

Merge v11#11
did-g wants to merge 78 commits intodavidkohn88:masterfrom
did-g:merge_v11

Conversation

@did-g
Copy link

@did-g did-g commented Nov 13, 2018

Hi,
Great
For reference what I did
most changes have an associated PG git ref. ie if you checkout PG and timescale, both should compile (maybe with some #if 0 help in compat.h).

there's at least 3 big issues
ad7dbee368a7cd9e595d2a957be784326b08c943
Date: Fri Feb 16 21:17:38 2018 -0800
Allow tupleslots to have a fixed tupledesc
Likely some memory freed to early.

Related but JIT off only

In PG have to revert:
Revert "Allow direct lookups of AppendRelInfo by child relid"
This reverts commit 7d872c91a3f9d49b56117557cdbb0c3d4c620687
TS doesn't properly set the cache.
==25292== at 0x57352B: find_childrel_parents (relnode.c:1242) NULL
==25292== by 0x518B61: generate_implied_equalities_for_column (equivclass.c:2229)
==25292== by 0x51C56D: match_eclass_clauses_to_index (indxpath.c:2173)
==25292== by 0x51958B: create_index_paths (indxpath.c:293)
==25292== by 0x5069DB: set_plain_rel_pathlist (allpaths.c:714)
==25292== by 0x506589: set_rel_pathlist (allpaths.c:452)
==25292== by 0x507AA0: set_append_rel_pathlist (allpaths.c:1340)
==25292== by 0x5064E9: set_rel_pathlist (allpaths.c:432)
==25292== by 0x5061C3: set_base_rel_pathlists (allpaths.c:310)
==25292== by 0x505F30: make_one_rel (allpaths.c:180)
==25292== by 0x539871: query_planner (planmain.c:265)
==25292== by 0x53C900: grouping_planner (planner.c:1901)
==25292== by 0x53AF8F: subquery_planner (planner.c:966)
==25292== by 0x539C53: standard_planner (planner.c:405)
==25292== by 0x10310E63: timescaledb_planner (planner.c:259)

RobAtticus and others added 30 commits October 18, 2018 18:01
This change moves all time_bucket-related functions to the same source
file (time_bucket.c) for consistency. There are no changes to code
logic.
The docker-build.sh script, which is used to build a Docker image from
the currently checked-out source code, has been refactored to make
better use of cached images. This speeds up the build process and allows
faster Travis builds.
This change makes the update tests from each prior version of
TimescaleDB to run in parallel instead of in serial order.

This speeds up the testing significantly.
Adaptive chunking is in beta, and we don't want users enabling it in
production by accident. This commit adds a warning to that effect.
Using DatumGetTimestampTz is requires on some 32-bit platforms under
PG 9.6.
Previously, the scheduler only populated its jobs list once at start time. This commit enables the scheduler to receive notifications for updates (insert, update, delete) to the bgw_job table. Notifications are sent via the cache invalidation framework. Whenever the scheduler receives a notification, it re-reads the bgw_job table. For each job currently in the bgw_job table, it either instantiates new scheduler state for the job or copies over any existing scheduler state, for persisting jobs. For jobs that have disappeared from the bgw_job table, the scheduler deletes any local state it has.

Note that any updates to the bgw_job table must now go through the C, so that the cache invalidation framework in catalog.c can run. In particular, this commit includes a rudimentary API for interacting with the bgw_job table, for testing purposes. This API will be rewritten in the future.
This change adds proper result types for the scanner's filter and
tuple handling callbacks. Previously, these callbacks were supposed to
return bool, which was hard to interpret. For instance, for the tuple
handler callback, true meant continue processing the next tuple while
false meant finish the scan. However, this wasn't always clear. Having
proper return types also makes it easier to see from a function's
signature that it is a scanner callback handler, rather than some
other function that can be called directly.
Adding ORDER BY to some select statements in cluster.sql test to ensure determinism.
Surprisingly, the compiler didn't complain about allocating a stack array with a size only known at runtime. Modified memory allocation to be heap-allocation to remedy this error.
If IN/ANY/ALL operator is used with explicit values
 we can effectively restrict chunks that need to be scanned.
Here are some examples of supported queries:
- SELECT * FROM hyper_with_space_dim WHERE time < 10 AND device_id IN ('dev5','dev6','dev7','dev8');
- SELECT * FROM hyper_with_space_dim WHERE device_id = ANY(ARRAY['dev5','dev6']) AND device_id = ANY(ARRAY['dev6','dev7']);

There are som cases that are not optimized:
- subqueries within IN/ANY/ALL
- open dimension (eg. time)  when using IN/ANY with multiple args
- NOT operator
In a database without timescaledb installed drop owned
failed with an error while trying to lookup the extension
owner.
snprint has different behavior on some windows versions than on unix.
This commit replaces the snprintf version with one that uses sscanf,
which behaves the same on all platforms. (This is a higher priority
commit than it would appear, since we're tracking down a stack-overflow,
and this is one of the candidates f
After this commit AppVeyor will run all TimescaleDB tests on windows
did-g added 28 commits November 21, 2018 08:37
Author: Peter Eisentraut <peter_e@gmx.net>
Date:   Sat Dec 2 09:26:34 2017 -0500

	Replace AclObjectKind with ObjectType

	'By using ObjectType instead, we can also give some more
    	precise error messages, for example "index" instead of "relation".'
commit 8237f27b504ff1d1e2da7ae4c81a7f72ea0e0e3e
Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date:   Mon Feb 12 19:30:30 2018 -0300

    get_relid_attribute_name is dead, long live get_attname
…a fixed tupledesc

XXX FIXME upsert

commit ad7dbee368a7cd9e595d2a957be784326b08c943
Author: Andres Freund <andres@anarazel.de>
Date:   Fri Feb 16 21:17:38 2018 -0800

	Allow tupleslots to have a fixed tupledesc
commit 04700b685f31508036456bea4d92533e5ceee9d6
Author: Peter Eisentraut <peter_e@gmx.net>
Date:   Fri Feb 16 20:44:15 2018 -0500

    Rename TransactionChain functions
commit 7a50bb690b4837d29e715293c156cff2fc72885c
Author: Andres Freund <andres@anarazel.de>
Date:   Fri Mar 16 23:13:12 2018 -0700

    Add 'unit' parameter to ExplainProperty{Integer,Float}.
PG:
commit 432bb9e04da4d4a1799b1fe7c723b975cb070c43
Author: Andres Freund <andres@anarazel.de>
Date:   Wed Mar 21 19:28:28 2018 -0700

    Basic JIT provider and error handling infrastructure.
…73b0ec5b0f27066e37dd93a7f0a96

commit ee0a1fc84eb29c916687dc5bd26909401d3aa8cd (HEAD)
Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date:   Mon Mar 19 18:09:43 2018 -0300

    Remove unnecessary members from ModifyTableState and ExecInsert
commit 86f575948c773b0ec5b0f27066e37dd93a7f0a96
Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date:   Fri Mar 23 10:48:22 2018 -0300

    Allow FOR EACH ROW triggers on partitioned tables
XXX rowsecurity-10

commit 555ee77a9668e3f1b03307055b5027e13bf1a715
Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date:   Mon Mar 26 10:43:54 2018 -0300

    Handle INSERT .. ON CONFLICT with partitioned tables
commit 16828d5c0273b4fe5f10f42588005f16b415b2d8 (HEAD)
Author: Andrew Dunstan <andrew@dunslane.net>
Date:   Wed Mar 28 10:43:52 2018 +1030

    Fast ALTER TABLE ADD COLUMN with a non-NULL default
commit 7e0d64c7a57e28fbcf093b6da9310a38367c1d75
Author: Robert Haas <rhaas@postgresql.org>
Date:   Mon Apr 2 10:51:50 2018 -0400

    postgres_fdw: Push down partition-wise aggregation.
commit eed1ce72e1593d3e8b7461d7744808d4d6bf402b
Author: Magnus Hagander <magnus@hagander.net>
Date:   Thu Apr 5 18:59:32 2018 +0200

    Allow background workers to bypass datallowconn
commit c9c875a28fa6cbc38c227fb9e656dd7be948166f (HEAD)
Author: Teodor Sigaev <teodor@sigaev.ru>
Date:   Thu Apr 12 13:02:45 2018 +0300

    Rename IndexInfo.ii_KeyAttrNumbers array
commit fb466d7b5dbe73f318324cada80203522f46401f
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Tue Sep 4 13:45:35 2018 -0400

    Fully enforce uniqueness of constraint names.
after postgres commit c09daa9104099422ea998e0398934ca82eb37898 'fatal' is either not valid
or replaced by 'error'.
davidkohn88 pushed a commit that referenced this pull request Nov 2, 2019
Whenever paths are added to a rel, that path or another
path that previously was on the rel can be freed.

Previously, the compressed rel's paths could be freed when it was re-planned
by the postgres planner after being created and planned by us. The new path
the postgres planner added was cheaper and overwrote and pfreed the old path
which we created and saved as a child path of the decompress node. Thus
we ended up with a dangling reference to a pfreed path.

This solution prevents this bug by removing the path we create from the
compressed rel. Thus, the chunk rel now "owns" the path. Note that this
does not prevent the compressed rel from being replanned and thus some
throw-away planner work is still happening. But that's a battle for
another day.

The backtrace for the core planner overwriting our path is:
```
frame #4: 0x0000000105c4ed0f postgres`pfree(pointer=0x00007fe20d01a628) at mcxt.c:1035
  * frame #5: 0x000000010594c998 postgres`add_partial_path(parent_rel=0x00007fe20d01ae10, new_path=0x00007fe20f800298) at pathnode.c:844
    frame #6: 0x00000001058ede4b postgres`create_plain_partial_paths(root=0x00007fe2113fc668, rel=0x00007fe20d01ae10) at allpaths.c:753
    frame #7: 0x00000001058edb93 postgres`set_plain_rel_pathlist(root=0x00007fe2113fc668, rel=0x00007fe20d01ae10, rte=0x00007fe20d0198c0) at allpaths.c:727
    frame #8: 0x00000001058ed78b postgres`set_rel_pathlist(root=0x00007fe2113fc668, rel=0x00007fe20d01ae10, rti=13, rte=0x00007fe20d0198c0) at allpaths.c:452
    frame #9: 0x00000001058e8e16 postgres`set_base_rel_pathlists(root=0x00007fe2113fc668) at allpaths.c:310
    frame #10: 0x00000001058e8b49 postgres`make_one_rel(root=0x00007fe2113fc668, joinlist=0x00007fe20d0121c8) at allpaths.c:180
    frame #11: 0x000000010591ee77 postgres`query_planner(root=0x00007fe2113fc668, tlist=0x00007fe2113fcb58, qp_callback=(postgres`standard_qp_callback at planner.c:3492), qp_extra=0x00007ffeea6ba2b8) at planmain.c:265
    frame #12: 0x00000001059229cb postgres`grouping_planner(root=0x00007fe2113fc668, inheritance_update=false, tuple_fraction=0) at planner.c:1942
    frame #13: 0x0000000105920546 postgres`subquery_planner(glob=0x00007fe218000328, parse=0x00007fe218000858, parent_root=0x0000000000000000, hasRecursion=false, tuple_fraction=0) at planner.c:966
    frame timescale#14: 0x000000010591f1e7 postgres`standard_planner(parse=0x00007fe218000858, cursorOptions=256, boundParams=0x0000000000000000) at planner.c:405
    frame timescale#15: 0x000000010642d9b4 timescaledb-1.5.0-dev.so`timescaledb_planner(parse=0x00007fe218000858, cursor_opts=256, bound_params=0x0000000000000000) at planner.c:152
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants