It was quite clearly broken, as it didn't do anything with the new_tle
that it chased up, and the end result was just the same as calling
canonicalize_pathkeys() on the input.

I'm not 100% the code is doing the right thing even with this: If the
pathkey's operator family doesn't match the join clause's operator family,
the output isn't truly ordered the same way. This is not an issue for the
case that I included in the regression test, where the join happens on a
constant, but I can't figure out whether this might be an issue with some
other query.

The comment in apply_shareinput_xslice() said that we process top-plan
first, to ensure that we find the producer of a shared scan before its
consumers, when the top plan contains a producer for subplans. But the
situation can just as well be the other way 'round.

No new test case added, because running the existing with_clause test
with gp_cte_sharing=on trips the assertion.

Fixes github issue #287.

My previous commit (3d0a4806), which was solving the issue described in pull request #276, was braking some ICG tests in some operating systems and we reverted it. This is a complete fix for the same issue.

This reverts commit 3d0a4806.

Closes #288

This fix prevents gpdb from a segmentation fault in case that: i) a SharedInputScan has a Sort child node, ii) sort spills, and iii) somebody requests to finish the execution during the last stage of the sort (i.e., QueryFinishPending = true). Currently, in the above scenario, SharedInputScan continues the execution and tries to access sort's result_tape, which is NULL, due to QueryFinishPending.

This commit makes a sort node to pretend that there are no results when QueryFinishPending flag is set to true. The commit also  creates a new Fault injector type and includes an ICG test.

int2vector and aclitem don't have B-tree operators, making them not
mergejoinable. In the merge of the equivalence classes patch, I made
cdb_make_pathkeys_for_expr() more strict on that, and it now throws
an error if it's passed a datatype that doesn't have a mergejoinable
= operator. It used to return a path key with InvalidOid as the sort
operator, but that seems like a recipe for bugs elsehere.

To fix, mandate that all gpdb-hashable datatypes must be mergejoinable.
int2vector and aclitem are not, so forbid using them in DISTRIBUTED BY.

This is a backwards-incompatible change, but hopefully no-one is using
int2vector or aclitem in user-tables, at least not as a distribution key.

This is dead, because SubPlan is an Expr node and therefore cannot appear
in a Plan tree.

This was broken, because the "subplan = (SubPlan *) subplan" cast didn't
do anything, and subplan was potentially uninitialized. It meant to say
"subplan = (SubPlan *) plan".

Spotted by Daniel Gustafsson.

If there is a schema in the database that the user does not have access to,
doing "select * from gp_toolkit.__gp_user_tables_readable" throws a
permission denied error on the schema. Use the relation-OID-variant of
has_table_privilege() function, to work around that.

This isn't otherwise better than the existing code, but this matches the
upstream more closely, which will make merging slightly easier.

parser/analyze.c is bloated, moving larger chunks of GPDB-specific
functionality like this elsewhere makes it more readable. Make diffing and
merging of analyze.c with upstream easier, too.

I also ran pgindent on parse_partition.c.

parser/analyze.c is overcrowded, and since it's an upstream file, extra
code there can confuse merging (though completely new functions are less
of a problem).

The GPDB-specific auto-stats system doesn't really have anything to do with
autovacuum. It's not run by the autovacuum daemon, and there are no calls
from the autovacuum code to autostats code.

Since we are using 64 bit atomic operations on this pointer, it requires
to be 64-bit aligned. This only affects OS X builds.

Closes #269.

gp_read_error_log and gp_truncate_error_log were two functions which were added
in 4.3.3.0 to support the new error logging functionality for external tables.
They were added to a "hidden" catalog in order to prevent bumping up the
catalog version in a minor version release. We need to add them to the true
catalog in a major release.

Also in the process remove hiddencat mechanism since apart from the above two
functions no other functions use it.

Every 100th AO or CO row insert in a transaction leaks 64 bytes
(gpgastsequence tuple) memory. Hence for transaction performing bulk
inserts to AO or CO table memory footprint keeps increasing, without
the fix.

Moving the installation of gp_toolkit.sql into initdb, in commit f8910c3c,
broke all the functions that are supposed to execute on all nodes, like
gp_toolkit.__gp_localid. After that change, gp_toolkit.sql was executed
in utility mode, and the gp_distribution_policy entries for those functions
were not created as a result.

To fix, change the code so that gp_distribution_policy entries are never
never created, or consulted, for EXECUTE-type external tables. They have
more fine-grained information in pg_exttable.location field anyway, so rely
on that instead. With this change, there is no difference in whether an
EXECUTE-type external table is created in utility mode or not. We would
still have similar problems if gp_toolkit contained other kinds of
external tables, but it doesn't.

This removes the isMasterOnly() function and changes all its callers to
call GpPolicyFetch() directly instead. Some places used GpPolicyFetch()
directly to check if a table is distributed, so this just makes that the
canonical way to do it. The check for system schemas that used to be in
isMasterOnly() are no longer performed, but they should've unnecessary in
the first place. System tables don't have gp_distribution_policy entries,
so they'll be treated as master-only even without that check.

For "checksum=on/off", we used the "on" or "off" value to allocate space
(i.e. 2 or 3 bytes), but then constructed a string that said "true" or
"false" (4 or 5 bytes). That lead to overflowing the allocation by a few
bytes, which produced warnings on assertion-enabled builds.

The integer parsing almost has the same bug. We use the original string's
length to allocate space, but we use %d to construct the value actually
stored there. That happens to not be a bug, because the original length is
always at least as large. This is a near-miss though: the original value
might be in hexadecimal format, i.e. "0x7fffffff", and its decimal
representation might be longer than the original. With 32-bit integers,
however, that happens to not be possible. as the extra "0x" in front of the
hex string provides enough padding. (i.e. strlen("0x7fffffff") >=
strlen("2147483647)"))

To fix, don't even try to count the space needed. Use a StringInfo instead.

In the passing, clean up accumAOStorageOpt function in some other ways:
remove the namelen argument to make it a bit more readable (this is hardly
performance-critical code), and don't split error message across lines
(makes it easier to grep for them if you need to find where an error
is coming from).

Usage: minirepro <database> [options]

Options:
  --version             show program's version number and exit
  -?, --help            Show this help message and exit
  -h HOST, --host=HOST  Specify a remote host
  -p PORT, --port=PORT  Specify a port other than 5432
  -U USER, --user=USER  Connect as someone other than current user
  -q QUERY_FILE         file name that contains the query
  -f OUTPUT_FILE        minirepro output file name

Closes #157.

Remove superflous ";" to ensure that the return value is set based
on the success of udpSignalGet(). Verified by lchang@.

This only affects OS X builds.

Adding functionality to enable retrieving the number of evicted objects from MDCache for testing purposes

This table is not used anywhere in the code.

Introduced by a89b2684.

These were both only used by a single, static, function.

Upstream doesn't have them, and they are just plain setter/getter
functions. Having functions like this woldn't be a bad idea per se, but
will cause merge conflicts.

This removes a couple of UnusedArg() calls. I'm not getting a compiler
warning from that function, and upstream seems to be happy without them
too.

It's unclear which upstream version our copy of this code is based on.
Looks like we have some of the changes from upstream commit b9527e98, but
not all. In any case, no upstream version has a variable called
'plannedstmt' here, all versions that have that variable call it 'pstmt'.

Adding a new 'return' like this in CheckAttributeType() is a bit risky, but
it seems better than re-indenting the whole function. Looking ahead at the
upcoming patches we're merging, we want to short-circuit all future checks
in this function too. In general, the reasoning is that any checks we might
want to perform on the attribute type should've been performed on the Query
Dispatcher already, and Query Executors should just follow orders.

The real ones are in parse_expr.h and makefuncs.h.

My compiler was complaining that the newResult variable might be passed
to pfree() uninitialized. I didn't spend too much time investigating that,
however. Instead, I replaced the whole FuncnameGetCandidates() function
with the PostgreSQL 8.4 version, and then modified it so that it works
without default-parameter support, which we haven't backported. Keeping
the code as close as possible to the upstream version seems like a good
idea. This also fixed some spurious whitespace differences vs. the upstream
version.

In the passing, clean up the regression test a bit. There are no
replacement strings in it, so move it from input/output to sql/expected
directories, and also add a comment pointing out that the tests are
backported from other PostgreSQL tests.