Harden core APIs by adding the `const` qualifier to pointer parameters
and return values passed by reference. Adding `const` to APIs has
several benefits and potentially reduces bugs.
* Allows core APIs to be called using `const` objects.
* Callers know that objects passed by reference are not modified as a
side-effect of a function call.
* Returning `const` pointers enforces "read-only" usage of pointers to
internal objects, forcing users to copy objects when mutating them
or using explicit APIs for mutations.
* Allows compiler to apply optimizations and helps static analysis.
Note that these changes are so far only applied to core API
functions. Further work can be done to improve other parts of the
code.
The TriggerDesc from rel->trigdesc seems to be modified during
iterations of the loop and sometimes gets reallocated leading
to a situation where the local variable trigdesc no longer matches
rel->trigdesc leading to a segfault in create_trigger_handler.
Fixes#2013
This patch removes code support for PG9.6 and PG10. In addition to
removing PG96 and PG10 macros the following changes are done:
remove HAVE_INT64_TIMESTAMP since this is always true on PG10+
remove PG_VERSION_SUPPORTS_MULTINODE
In distributed hypertables, chunks are foreign tables and such tables
do not support (or should not support) indexes, certain constraints,
and triggers. Therefore, such objects should not recurse to foreign
table chunks nor add a mappings in the `chunk_constraint` or
`chunk_index` tables.
This change ensures that we properly filter out the indexes, triggers,
and constraints that should not recurse to chunks on distributed
hypertables.
The internal chunk API is updated to avoid returning `Chunk` objects
that are marked `dropped=true` along with some refactoring, hardening,
and cleanup of the internal chunk APIs. In particular, apart from
being returned in a dropped state, chunks could also be returned in a
partial state (without all fields set, partial constraints,
etc.). None of this is allowed as of this change. Further, lock
handling was unclear when joining chunk metadata from different
catalog tables. This is made clear by having chunks built within
nested scan loops so that proper locks are held when joining in
additional metadata (such as constraints).
This change also fixes issues with dropped chunks that caused chunk
metadata to be processed many times instead of just once, leading to
potential bugs or bad performance.
In particular, since the introduction of the “dropped” flag, chunk
metadata can exist in two states: 1. `dropped=false`
2. `dropped=true`. When dropping chunks (e.g., via `drop_chunks`,
`DROP TABLE <chunk>`, or `DROP TABLE <hypertable>`) there are also two
modes of dropping: 1. DELETE row and 2. UPDATE row and SET
dropped=true.
The deletion mode and the current state of chunk lead to a
cross-product resulting in 4 cases when dropping/deleting a chunk:
1. DELETE row when dropped=false
2. DELETE row when dropped=true
3. UPDATE row when dropped=false
4. UPDATE row when dropped=true
Unfortunately, the code didn't distinguish between these cases. In
particular, case (4) should not be able to happen, but since it did it
lead to a recursing loop where an UPDATE created a new tuple that then
is recursed to in the same loop, and so on.
To fix this recursing loop and make the code for dropping chunks less
error prone, a number of assertions have been added, including some
new light-weight scan functions to access chunk information without
building a full-blown chunk.
This change also removes the need to provide the number of constraints
when scanning for chunks. This was really just a hint anyway, but this
is no longer needed since all constraints are joined in anyway.
relation_open is a general function, which is called from more
specific functions per database type. This commit replaces them
with the specific functions, which control correct types.
This change enables a check in `create_hypertable` that prohibits
turning partitioned tables into hypertables. The check was only
enabled when compiling against PG10, but should be there for PG
version 10 and greater.
To avoid such disabled code in the future, some extra convenience
macros have been added. For instance `PG10_GE` means PG10 and greater.
To create a continuous agg you now only need SELECT and
TRIGGER permission on the raw table. To continue refreshing
the continuous agg the owner of the continuous agg needs
only SELECT permission.
This commit adds tests to make sure that removing the
SELECT permission removes ability to refresh using
both REFRESH MATERIALIZED VIEW and also through a background
worker.
This work also uncovered divergence in permission logic for
creating triggers by a CREATE TRIGGER on chunks and when new
chunks are created. This has now been unified: there is a check
to make sure you can create the trigger on the main table and
then there is a check that the owner of the main table can create
triggers on chunks.
Alter view for continuous aggregates is allowed for the owner of the
view.
Introduce PG11 support by introducing compatibility functions for
any whose signatures have changed in PG11. Additionally, refactor
the structure of the compatibility functions found in compat.h by
breaking them out by function (or small set of similar functions)
so that it is easier to see what changed between versions and maintain
changes as more versions are supported.
In general, the philosophy has been to try for forward compatibility
wherever possible, so that we use the latest versions of function interfaces
where we can or where reasonably convenient and mimic the behavior
in older versions as much as possible.
Future proofing: if we ever want to make our functions available to
others they’d need to be prefixed to prevent name collisions. In
order to avoid having some functions with the ts_ prefix and
others without, we’re adding the prefix to all non-static
functions now.
Previously, automatic chunk creation on INSERT failed due to lack of
permissions when the hypertable had triggers that needed to be
replicated to the new chunk. This happened because creating triggers
on tables requires TRIGGER permission, and the internal code used
CreateTrigger() that performs permissions checks. Thus, if the user
inserting tuples into the hypertable only had INSERT permissions, the
insert would fail whenever a new chunk was created.
This change fixes the issue by temporarily assuming the role of the
hypertable owner when executing CreateTrigger() on the new chunk.
Source code indentation has been updated in PostgreSQL 10 to fix a
number of issues. This update applies this new indentation to the
entire code base.
The new indentation requires a new version of pg_bsd_indent, which can
be found here:
https://git.postgresql.org/git/pg_bsd_indent.git
The extension now works with PostgreSQL 10, while
retaining compatibility with version 9.6.
PostgreSQL 10 has numerous internal changes to functions and
APIs, which necessitates various glue code and compatibility
wrappers to seamlessly retain backwards compatiblity with older
versions.
Test output might also differ between versions. In particular,
the psql client generates version-specific output with `\d` and
EXPLAINs might differ due to new query optimizations. The test
suite has been modified as follows to handle these issues. First,
tests now use version-independent functions to query system
catalogs instead of using `\d`. Second, changes have been made to
the test suite to be able to verify some test outputs against
version-dependent reference files.
Applying triggers to chunks requires taking the definition
of a trigger on a hypertable and executing it on a chunk. Previously
this was done with string replacement in the trigger definition.
This was not especially safe, and thus we moved the logic to C
where we can do proper parsing/deparsing and replacement of the table
name. Another positive aspect is that we got rid of some DDL triggers.
