The calculation for minimum amount of memory to memory intensive
operators was using kB for both input and output, leading to an
assertion failure in case the minimum memory requirement was less
than a kB. Since we are using a full uint64 for calculating, we
have room for using just bytes instead leaving the formatting to
the caller (which is just EXPLAIN ANALYZE).
Reviewed-by: NWeinan Wang <wewang@pivotal.io>

While hacking on another thing, I spotted that we had numerous lines
with tab + 1 space indentation, which is clearly wrong. Fix by using
just tab indentation as per the style guide.

* docs - GDD SELECT...FOR locking

Add information about GDD managing SELECT...FOR <locking strength> queries

Currently this feature is in MASTER only.

* docs - updates from review comments

* docs - fix typo

Adds tablespace OID to pg_xlogdump output for xl_xact_commit and xl_xact_abort records for debugging purposes. The OID information is actually redundant and can be backtraced through the commit record, however it could still be useful in case the first message is not available.
Co-authored-by: NIvan Leskin <leskin.in@arenadata.io>

This commit adds the GPDB side changes required to support GIN Indexes with
ORCA.  It also adds a new test file gp_gin_indexes to test plans produced for
ORCA/planner.

GIN indexes are not supported with index expression or predicate constraints.
ORCA does not support it currently for other types of indexes too.

There is a pipeline for unit tests in GPDB used in most cases.

However, unit tests of src/backend/access/gin introduced by 99360f54
used a custom implementation of unit test build script. This led to
errors e.g. when a compiler different than GCC was used to build GPDB.

Rewrite Makefile in order to unify test infrastructure with common
pattern used in the backend while retaining test isolation from the
backend objects.

See also similar Makefile: src/backend/catalog/test/Makefile
at 122c79f2

Note the Makefile in src/backend/access/gin/test is different from
currently most used version of a backend unit test Makefile. These
differences and motivation for them is described in the README.

Run pgindent on ginpostlist_fakes.c
Reviewed-by: NAdam Berlin <aberlin@pivotal.io>
Reviewed-by: NDaniel Gustafsson <dgustafsson@pivotal.io>
Reviewed-by: NHeikki Linnakangas <hlinnakangas@pivotal.io>
Co-authored-by: NIvan Leskin <leskin.in@arenadata.io>

The following SQL snippet demonstrates dependencies on array types derived from partitions of partitioned tables:

```
CREATE TABLE table_part(a INT, b INT) PARTITION BY RANGE(b) (PARTITION part1 START(0) END(42));
CREATE TABLE replacement(LIKE table_part);
ALTER TABLE table_part EXCHANGE PARTITION part1 WITH TABLE replacement;
CREATE TABLE dependant(d table_part_1_prt_part1[]);
```

Such dependencies cannot be carried over to the upgraded cluster, and pg_upgrade should fail when they are present. The user should take extra steps to resolve the situation.

When check fails, the output is similar to the following:

```
Checking array types derived from partitions                fatal

Array types derived from partitions of a partitioned table must not have dependants.
OIDs of such types found and their original partitions:
16393 public.table_part_1_prt_part1
16471 public.table_part2_1_prt_part1
Failure, exiting
```
Reviewed-by: NSoumyadeep Chakraborty <sochakraborty@pivotal.io>
Co-authored-by: NIvan Leskin <leskin.in@arenadata.io>

ALTER TABLE DROP COLUMN followed by reorganize leads to loss of column
encoding settings of the dropped column. When the column's compresstype
encoding is incorrect, we can encounter block version mismatch error
later during block info validation check of the dropped column.

One idea was to skip dropped columns when constructing AOCSScanDesc.
However, dropping all columns is a special case that is not easily handled
because it is not equivalent to deleted rows. Instead, the fix is to preserve
column encoding settings even for dropped columns.
Co-authored-by: NSoumyadeep Chakraborty <sochakraborty@pivotal.io>
Co-authored-by: NIvan Leskin <leskin.in@arenadata.io>

The big gpMgmt Makefile rewrite in b5aba18b
was a paddle short for make distclean in gpMgmt/bin/lib as it was
missing the 'rm' invocation.

Some callers of pg_mkdir_p() consider EEXIST as a non-error case, this
is incorrect.  When pg_mkdir_p() sets errno to EEXIST it means that the
target path already exists but is not a directory.  So as long as
pg_mkdir_p() returns -1 we should consider it as a failure.
Co-authored-by: NPaul Guo <pguo@pivotal.io>
Co-authored-by: NNing Yu <nyu@pivotal.io>

pg_mkdir_p() is used by initdb and pg_basebackup to create a directory
as well as its parent directories.  The logic used to be like below:

    if (stat(path) < 0)
    {
        if (mkdir(path) < 0)
            retval = -1;
    }

It first checks for the existence of the path, if path does not
pre-exist then it creates the directory.  However if two processes try
to create path concurrently, then one possible execution order is as
below:

    A: stat(path) returns -1, so decide to create it;
    B: stat(path) returns -1, so decide to create it;
    B: mkdir(path) returns 0, successfully created path;
    A: mkdir(path) returns -1, fail to create path as it already exist;

This race condition could be fixed by swapping the order of stat() and
mkdir(), this is also what the "mkdir -p" command does.
Co-authored-by: NPaul Guo <pguo@pivotal.io>
Co-authored-by: NNing Yu <nyu@pivotal.io>

package_tarball.bash is used in many places outside of the gpdb
repository so any modifications should be done very carefully.

This commit also introduces package_tarball.bats as an example file for
other concourse users wishing to add .bats files.
Authored-by: NAmil Khanzada <akhanzada@pivotal.io>
Reviewed-by: NMark Sliva <msliva@pivotal.io>

We have added tests to ensure that the search_path setting is taking effect. We
also are using a test database for the tests.
Co-authored-by: NJamie McAtamney <jmcatamney@pivotal.io>
Co-authored-by: NJacob Champion <pchampion@pivotal.io>
Co-authored-by: NNikolaos Kalampalikis <nkalampalikis@pivotal.io>

The unsetting of search_path started an implicit transaction that can
affect the behavior of subsequent SQL statements. A simpler solution
(thanks Heikki!) is to just set the search_path for the entire
connection using the connection string options.
Co-authored-by: NJamie McAtamney <jmcatamney@pivotal.io>
Co-authored-by: NJacob Champion <pchampion@pivotal.io>
Co-authored-by: NNikolaos Kalampalikis <nkalampalikis@pivotal.io>

GUC gp_enable_gpperfmon is defined to be set only at postmaster
restart.  Having a check hook that checks if the process setting it
has superuser privileges is meaningless.  The check hook is removed.

GUC gp_gpperfmon_send_interval is intended to be set only by
superuser.  Adjust its definition accordingly and leverage checks
built into GUC framework for superuser privileges.  The check hook for
this GUC tried to achieve the same but incorrectly.  If the check hook
was invoked at the beginning of main query processing loop by a
backend process, it would crash.  At the beginning of the main loop, a
transaction is not started yet.  The check hook invokes superuser()
interface, which performs catalog access.  Doing so without starting a
transaction is a recipe for crashing badly.  Such a crash was observed
in production at least once.

Thank you Jesse Zhang for suggesting to remove superuser check.

The patch doesn't add any tests because, after removing the check
hooks, the checks built into GUC framework are being used.  That code
path is well exercised by existing regression tests.

Reviewed-by: Daniel Gustafsson

Happened to spot these due to recent correspondence.

With the revamp of the gpMgmt Makefiles, the catalog json and stream
binary are now compiled on make all, so we should perform them there
rather than opt for building on make install. This still leaves some
modules which are compiled on make install, such as pygresql, but a
small start is still a start.

Discussion: https://github.com/greenplum-db/gpdb/pull/8179
Reviewed by Bradford Boyle, Kalen Krempely, Jamie McAtamney and
many more

The psi package is no longer installed anywhere, so remove the target
from the Makefile.

Reported-by: Shaoqi Bai
Discussion: https://github.com/greenplum-db/gpdb/pull/8179
Reviewed by Bradford Boyle, Kalen Krempely, Jamie McAtamney and
many more

Discussion: https://github.com/greenplum-db/gpdb/pull/8179
Reviewed by Bradford Boyle, Kalen Krempely, Jamie McAtamney and
many more

The stream code lacked prototypes proper enough to be allowed through
a modern compiler with sane compiler flags, fix by defining the funcs
as taking void parameter.

Discussion: https://github.com/greenplum-db/gpdb/pull/8179
Reviewed by Bradford Boyle, Kalen Krempely, Jamie McAtamney and
many more

Installing the Management utilities used to be pretty brute-force
operation which copied more or less everything over blindly and then
tried to remove what shouldn't be installed. This is clearly not a
terribly clean and sustainable solution, as subsequent issues with
it has proven (editor savefiles, patch .rej/.orig files etc were
routinely copied and never purged etc).

This takes a first stab at turning installation of gpMgmt/bin, sbin
and doc into proper recursive make targets which only install the
files that were intended to be installed.

Discussion: https://github.com/greenplum-db/gpdb/pull/8179
Reviewed by Bradford Boyle, Kalen Krempely, Jamie McAtamney and
many more

Break out compilation of stream from the gpMgmt/bin Makefile into its
own Makefile which follows the general structure for how programs are
compiled in the tree.  The resulting binary is installed in bin/lib
with a symlink to the previously used location.

This also moves stream from src/ as it was the only directory left in
there making the indirection pointless.

Discussion: https://github.com/greenplum-db/gpdb/pull/8179
Reviewed by Bradford Boyle, Kalen Krempely, Jamie McAtamney and
many more

gpfdist was rewritten in C from Python, but the old Python code was
never removed from the repo.

Discussion: https://github.com/greenplum-db/gpdb/pull/8179
Reviewed by Bradford Boyle, Kalen Krempely, Jamie McAtamney and
many more

The packcore version has been fixed to 0.4beta since 2014 when it
was bumped from 0.3alpha. As it is installed alongside other tools
lets version it with the main distribution.

Discussion: https://github.com/greenplum-db/gpdb/pull/8179
Reviewed by Bradford Boyle, Kalen Krempely, Jamie McAtamney and
many more

The sbindir is used in for some Python management utilities, but was
hardcoded rather than using the proper supplied variable. Exporting
from Makefile.global is a first step, next up is to fix all instances
where the directory name is hardcoded.

Discussion: https://github.com/greenplum-db/gpdb/pull/8179
Reviewed by Bradford Boyle, Kalen Krempely, Jamie McAtamney and
many more

The compiled python files ending with 'c' were only to some degree
removed on make clean, as the list in the Makefile hadn't been kept
in sync with the gitignore list.

Discussion: https://github.com/greenplum-db/gpdb/pull/8179
Reviewed by Bradford Boyle, Kalen Krempely, Jamie McAtamney and
many more

The .gitignore for gpMgmt had lots of entries for long since removed
applications. The apps and their respective delete commit were:

  * gp_df a7405a274c8ad392141ed4006adde8c506467af9
  * gpbitmapreindex e9b9081bf948f60aadf4f41a4e3f8ff43ce888ce
  * gpdbrestore & gpcrondump 8922315e
  * gpdebug 5ef6b86b
  * gpfilespace 5a3a39bc
  * gpkill 29f17dd8
  * gpmigrator 68178574

Discussion: https://github.com/greenplum-db/gpdb/pull/8179
Reviewed by Bradford Boyle, Kalen Krempely, Jamie McAtamney and
many more

And reset it after to make sure no memory leaks there.

For instance, the deserialized transValues for new entries (not that
temporary) are not in group_buf, and not freed.
Co-authored-by: NNing Yu <nyu@pivotal.io>

So we could account its usage via GET_TOTAL_USED_SIZE(hashtable) and
decide if it's time to spill.

And now it's not needed to pfree() the transValue since it's in mpool
and will be reset after spill.
Co-authored-by: NGang Xiong <gxiong@pivotal.io>
Co-authored-by: NRichard Guo <riguo@pivotal.io>

AggStates are now pointers allocated in aggcontext with type INTERNAL,
just spilling the pointers don't decrease the memory usage and have
possible memory leak if combining states without free.

This commit serialize the aggstates, write the real data into file and
free the memory.

* docs - can now reset pxf on host and cluster

* remove the (re)s

NON_PRODUCTION_FILES.txt is empty and now no longer used.
The external pipelines which use these scripts were also updated to
remove the files.
Co-authored-by: NTingfang Bao <bbao@pivotal.io>
Co-authored-by: NAmil Khanzada <akhanzada@pivotal.io>

In resource group mode we ever introduced an operator memory automatic
enlarging logic for hashagg, the point is to let hashagg fail on actual
OOM instead of a soft quote checking, this helps to let hashagg run
successfully with an initial low operator memory.

However a bug was introduced by the auto enlarging logic, hashagg
spilling can be disabled in resource group mode by accident.

On the other hand we introduced a memory_spill_ratio=0 mode in resource
group to use statement_mem for operators, which is the same behavior as
resource queue.  The statement_mem setting is usually large enough and
fine tuned by the users, in such a case we do not need the auto enlarge
for hashagg, and it is better to keep the old quote checking behavior.

In such a case we revert the hashagg related changes from below commits:

- 40d955d6 Rid resource group on hashagg spill evaluation (#8199)
- ede74cdc resgroup: reduce log level for operator memory overuse
- f053e6cd resgroup: allow memory overuse for hashagg spill meta data
- 90795402 resgroup: allow operators enlarge their memory quota
Reviewed-by: NAdam Lee <ali@pivotal.io>
Reviewed-by: NWeinan WANG <wewang@pivotal.io>

Discussion: https://groups.google.com/a/greenplum.org/d/msg/gpdb-dev/30hiTArxsgo/aydVMcrXBQAJ