Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherry pick from GPDB (02/07/2022-02/28/2022) #432

Merged
merged 24 commits into from
May 14, 2024

Conversation

avamingli
Copy link
Contributor

@avamingli avamingli commented May 13, 2024

Cherry-pick from GPDB from 02/07/2022 to 02/28/2022, except some commits:

  1. PG commits already in CBDB.
  2. GPDB docs or CI commits.
  3. Some invalid commits(Revert a non-exist commit in CBDB).

Fix conflicts and errors.


Change logs

Describe your change clearly, including what problem is being solved or what feature is being added.

If it has some breaking backward or forward compatibility, please clary.

Why are the changes needed?

Describe why the changes are necessary.

Does this PR introduce any user-facing change?

If yes, please clarify the previous behavior and the change this PR proposes.

How was this patch tested?

Please detail how the changes were tested, including manual tests and any relevant unit or integration tests.

Contributor's Checklist

Here are some reminders and checklists before/when submitting your pull request, please check them:

  • Make sure your Pull Request has a clear title and commit message. You can take git-commit template as a reference.
  • Sign the Contributor License Agreement as prompted for your first-time contribution(One-time setup).
  • Learn the coding contribution guide, including our code conventions, workflow and more.
  • List your communication in the GitHub Issues or Discussions (if has or needed).
  • Document changes.
  • Add tests for the change
  • Pass make installcheck
  • Pass make -C src/test installcheck-cbdb-parallel
  • Feel free to request cloudberrydb/dev team for review and approval when your PR is ready🥳

nmisch and others added 23 commits May 13, 2024 10:22
The back-patch of commit fdd965d074d46765c295223b119ca437dbcac973 broke
CLOBBER_CACHE_ALWAYS for v9.6 through v13.  It updated the
InvalidateSystemCaches() call for CLOBBER_CACHE_RECURSIVELY, neglecting
the one for CLOBBER_CACHE_ALWAYS.  Back-patch to v13, v12, v11, and v10.

Reviewed by Tomas Vondra.  Reported by Tomas Vondra.

Discussion: https://postgr.es/m/df7b4c0b-7d92-f03f-75c4-9e08b269a716@enterprisedb.com
In the function getResUsage it will set the group Id (type Oid)'s
value from a string. Previous code use pg_atoi to do the value parse,
this is not correct because type Oid is uint, we should use
atooid. Another place is building the SQL involving group id it uses
"%d", this commits fix this by using "%u".
ESubqueryCtxt indicates whether a subquery appears in the project list
(EsqctxtValue) or comparison predicate (EsqctxtFilter).  Currently we
can incorrectly recalculate the context inside PexprSubqueryPred().

Following query incorrectly determined the subquery was in the
comparison predicate:
    ```
    SELECT (SELECT 1)=ALL(SELECT generate_series(1,2));
    ```

Using the incorrect subquery context can cause ORCA to create a plan
that incorrect creates an inner (instead of left outer) correlated apply
operator. Ultimately, that mistake can lead to wrong results.

Fix is the pass the subquery context when available rather than
recalculate.
Given the following query:
```
SELECT 3 = ALL (SELECT generate_series(2, 3)) FROM (values (1),(2)) v(a);
```

Without predicate push down:
```
Physical plan:
+--CPhysicalComputeScalar
   |--CPhysicalCorrelatedLeftOuterNLJoin
   |  |--CPhysicalConstTableGet Columns: ["column1" (0)] Values: [(1); (2)]
   |  |--CPhysicalComputeScalar
   |  |  |--CPhysicalConstTableGet
   |  |  +--CScalarProjectList
   |  |     |--CScalarProjectElement "ColRef_0004" (4)
   |  |     |  +--CScalarConst (1)
   |  |     +--CScalarProjectElement "generate_series" (2)
   |  |        +--CScalarFunc (generate_series)
   |  |           |--CScalarConst (2)
   |  |           +--CScalarConst (3)
   |  +--CScalarCmp (=)
   |     |--CScalarConst (3)
   |     +--CScalarIdent "generate_series" (2)
   +--CScalarProjectList
      +--CScalarProjectElement "?column?" (3)
         +--CScalarIdent "ColRef_0004" (4)
```

With predicate push down:
```
Physical plan:
+--CPhysicalComputeScalar
   |--CPhysicalCorrelatedLeftOuterNLJoin
   |  |--CPhysicalConstTableGet Columns: ["column1" (0)] Values: [(1); (2)]
   |  |--CPhysicalComputeScalar
   |  |  |--CPhysicalFilter
   |  |  |  |--CPhysicalComputeScalar
   |  |  |  |  |--CPhysicalConstTableGet
   |  |  |  |  +--CScalarProjectList
   |  |  |  |     +--CScalarProjectElement "generate_series" (2)
   |  |  |  |        +--CScalarFunc (generate_series)
   |  |  |  |           |--CScalarConst (2)
   |  |  |  |           +--CScalarConst (3)
   |  |  |  +--CScalarCmp (=)
   |  |  |     |--CScalarConst (3)
   |  |  |     +--CScalarIdent "generate_series" (2)
   |  |  +--CScalarProjectList
   |  |     +--CScalarProjectElement "ColRef_0004" (4)
   |  |        +--CScalarConst (1)
   |  +--CScalarConst (1)
   +--CScalarProjectList
      +--CScalarProjectElement "?column?" (3)
         +--CScalarIdent "ColRef_0004" (4)
```

Both of these plans result in a SUBPLAN node of type ALL_SUBLINK. Inside
ExecScanSubPlan() executor will check that all evaulated expressions are
true. Issue is that if we push the predicate down and filter out all the
expressions that evaluate to false, then in example query we would
incorrectly return true from ExecSubPlan().

Fix is to avoid pushing down the predicate in the case of SUBPLAN of
type ALL_SUBLINK.
In order to support subplan test expressions with scalar ident params on
the left side of the test expression, we must allow more than one column
to be projected from the restricted result node beneath.
Pre-v13 versions only support logical replication from plain tables,
while v13 and later also allow partitioned tables to be published.
If you tried to subscribe an older server to such a publication,
you got "table XXX not found on publisher", which is pretty
unhelpful/confusing.  Arrange to deliver a more on-point error
message.  As commit c314c14 did in v13, remove the relkind check
from the query WHERE clause altogether, so that "not there"
is distinguishable from "wrong relkind".

Per report from Radoslav Nedyalkov.  Patch v10-v12.

Discussion: https://postgr.es/m/2952568.1644876730@sss.pgh.pa.us
Sometimes these prepared statements would be replanned and log ORCA
fallbacks, which are expected and ok. However, to make this more
deterministic, reset the plan cache in the test.
We saw a flake here, but haven't been able to repro. It's a fallback to
planner, but we're not exactly sure why. Hopefully this will give us a
hint if it happens again, and it's good to enable this so we can more
easily tell when and why Orca falls back.
… DISTRIBUTE

This fixes an issue where existing reloptions would cause a reorganize in
situations that it should not. We used to allow ALTER TABLE SET DISTRIBUTE WITH
clause to have additional storage options, and when they are present, we
will re-distribute data over the segments.

However since 50f2e3b, we don't allow new reloptions to be supplied in the
ALTER TABLE SET DISTRIBUTED WITH clause. So this commit is really a follow-up,
removing redistribution decision making logic based on reloptions.
Also renamed new_rel_opts() to get_rel_opts() accordingly.
Execution of multilevel correlated queries with high level of nesting
can cause segfault(when using array_agg, json_agg) or can provide wrong
results (when using classic aggs like sum()). Due to some GP
limitations, correlated subqueries with skip-level correlations are not
supported. Additional check condition is provided to prevent such
queries from planning. QueryHasDistributedRelation function, used by
this check, doesn't recurse over subplans and may return wrong results
for distributed RTE_RELATION entries hided by RTE_SUBQUERY entries.

Commit fixes such behavior by adding optional recursion to
QueryHasDistributedRelation function. Additional regression test is
included. Additional information can be found at issue #12054.
… uninitialized' (#13103)

* set to 0 with bzero to fix 'error: ‘data’ may be used uninitialized'

* change to memset for compatibiity

Co-authored-by: jasper li <ljasper@vmware.com>
When dealing with queries which have CTE and result set being used
more than once, ORCA produces a plan with Sequence Node and
ShareInputScan Node. Here is an example:

explain (costs off) with tmp_t as (select id from test) select a.id from tmp_t a join tmp_t b on a.id = b.id;
                         QUERY PLAN
------------------------------------------------------------
 Gather Motion 3:1  (slice1; segments: 3)
   ->  Sequence
         ->  Shared Scan (share slice:id 1:0)
               ->  Seq Scan on test
         ->  Hash Join
               Hash Cond: (share0_ref3.id = share0_ref2.id)
               ->  Shared Scan (share slice:id 1:0)
               ->  Hash
                     ->  Shared Scan (share slice:id 1:0)
 Optimizer: Pivotal Optimizer (GPORCA)
(10 rows)

This plan has three ShareInputScan nodes: one producer and two
consumers, all of them read from the tuplestore and return tuples to
upper node.  However, the CTE producer does not need to do so, because
the Sequence Node only receives the last subplan's output tuples.

When tuplestore is small, the extra read from the CTE producer is at
low cost, but when tuplestore is huge, this read is heavy and would
cause bad performance of the entire query.

This commit adds a flag discard_output in ShareInputScan plan node to
indicate when a ShareInputScan can avoid reading from tuplestore and
return early. The flag is set to true by ORCA when translating CTE producer,
and set to false elsewhere.

Fixes #12710

Co-authored-by: wuyuhao28 <wuyuhao28@github.com>
Co-authored-by: Alexandra Wang <walexandra@vmware.com>
Reviewed-by: Zhenghua Lyu <kainwen@gmail.com>
Reviewed-by: Shreedhar Hardikar <shardikar@vmware.com>
Using of regular MVCC snapshot to access AO-tables metadata may cause index building (performed globally) inconsistency, because tuples inserted (the same globally) at another opened transactions are not visible.

Commit fixes such behavior using SnapshotSelf for metadata access. Additional isolation test shows how to reproduce the bug before the fix.
This patch helps fix incorrect amount of memory allocated for WindowAgg.
Greenplum uses statement_mem rather than work_mem to control the memory
allocated to queries. Besides, test cases are attached for this patch.
aa7c74c - Support num_segments option for foreign servers and tables

Commit above introduces the support of num_segments option, however in
practice the option for foreign tables confuses the user.

This commit removes it from the foreign table layer, only the foreign
servers support.
It's out of date for quite a while, I also don't know if anyone uses
them for Greenplum development.
* Changes to avoid gpstop errors when standby is not reachable.

Added check for standby reachability before stopping the standby to avoid gpstop failures when standby is unreachable.
Added behave test case for gpstop when standby is not reachable.
Addressed review comments (Updated warning message)
… (#13144)

We observed greenplum built without cassert couldn't pass tests. Because
when the gpdb is configured with --enable-cassert, the minimum
statement_mem it accepts is 50kB while if it's configured without
--enable-cassert, the minimum statement_mem it accepts is 1000kB (See:
backend/utils/misc/guc_gp.c). This patch is trying to make pipelines
happy.
This patch is trying to make isolation2/parallel_retrieve_cursor/fault_inject
stable by replacing 'sleep' fault with 'suspend' fault.

Below is the regression.diffs file fetched from the PR pipeline.

```diff
diff -I HINT: -I CONTEXT: -I GP_IGNORE: -U3 /tmp/build/d62a0504/gpdb_src/src/test/isolation2/expected/parallel_retrieve_cursor/fault_inject.out /tmp/build/d62a0504/gpdb_src/src/test/isolation2/results/parallel_retrieve_cursor/fault_inject.out
--- /tmp/build/d62a0504/gpdb_src/src/test/isolation2/expected/parallel_retrieve_cursor/fault_inject.out	2022-02-24 16:18:00.128491568 +0000
+++ /tmp/build/d62a0504/gpdb_src/src/test/isolation2/results/parallel_retrieve_cursor/fault_inject.out	2022-02-24 16:18:00.940556278 +0000
@@ -684,6 +684,3256 @@
  READY
 (1 row)
 2R&: @pre_run 'set_endpoint_variable @ENDPOINT6': RETRIEVE ALL FROM ENDPOINT "@ENDPOINT6";  <waiting ...>
+FAILED:  Forked command is not blocking; got output: a
+-------
+ 5
+ 6
+ 9
+ ...
+ 9999
+ 10000
+(3247 rows)

 1U: SELECT state FROM gp_segment_endpoints() WHERE cursorname='c1';
  state
@@ -699,7 +3949,7 @@
 0R<:  <... completed>
 ERROR:  endpoint is not available because the parallel retrieve cursor was aborted (cdbendpointretrieve.c:LINE_NUM)
 2R<:  <... completed>
-ERROR:  endpoint is not available because the parallel retrieve cursor was aborted (cdbendpointretrieve.c:LINE_NUM)
+FAILED:  Execution failed

 1<:  <... completed>
 FAILED:  Execution failed
```
@CLAassistant
Copy link

CLAassistant commented May 13, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 14 committers have signed the CLA.

✅ avamingli
❌ fairyfar
❌ nmisch
❌ kainwen
❌ l-wang
❌ huansong
❌ tglsfdc
❌ dgkimura
❌ wuyuhao28
❌ lij55
❌ xiaoxiaoHe-E
❌ Annu149
❌ adam8157
❌ InnerLife0
You have signed the CLA already but the status is still pending? Let us recheck it.

@avamingli avamingli added the cherry-pick cherry-pick upstream commts label May 13, 2024
Some codes do not work in CBDB after Merge from GPDB.
Fix errors and etc.

Authored-by: Zhang Mingli avamingli@gmail.com
@my-ship-it my-ship-it merged commit 30da5b4 into apache:main May 14, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick cherry-pick upstream commts
Projects
None yet
Development

Successfully merging this pull request may close these issues.