Support ALTER TABLE SET DISTRIBUTED BY for external tables #818
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
my-ship-it
previously approved these changes
Dec 26, 2024
gfphoenix78
previously approved these changes
Dec 27, 2024
In Greenplum, different from upstream, will try to pull up the correlated EXISTS sublink which
has aggregate, just like:
```
postgres=# create table t(a int, b int);
postgres=# create table s (a int, b int);
postgres=# explain (costs off)
select * from t where exists (select sum(s.a) from s where s.a = t.a group by s.a);
QUERY PLAN
------------------------------------------
Gather Motion 3:1 (slice1; segments: 3)
-> Hash Join
Hash Cond: (t.a = s.a)
-> Seq Scan on t
-> Hash
-> HashAggregate
Group Key: s.a
-> Seq Scan on s
Optimizer: Postgres query optimizer
(9 rows)
```
So Greenplum changed the behavior of function `convert_EXISTS_sublink_to_join()` and
`simplify_EXISTS_query()`, so `simplify_EXISTS_query()` will simplify the EXISTS sublink
which has aggregate node, it will reset `targetList` and other clauses to null, just like:
```
query->targetList = NIL;
query->distinctClause = NIL;
query->sortClause = NIL;
query->hasDistinctOn = false;
```
But when we exec uncorrelated EXISTS sublink which also has an aggregate node, we can't
reset `targetList`. Otherwise, we can't make a plan for an aggregate node that doesn't have
an aggregate column, just like #11849.
This patch tries to fix the above issue, and the thought is **NOT** simplify uncorrelated EXISTS
sublink in some situation.
Creating AMs with internal handlers will not have entries in `pg_depend`, which is reasonable since internal handlers could not be dropped, but `gpcheckcat` is not happy with it. This commit excludes that and fixes the `getName()` deprecated issue.
This could be a good case for gpcheckcat testing access methods with internal handlers.
… remove macro IC_PROXY_LOG_LEVEL Currently ic_proxy_log(LOG would never log anything as IC_PROXY_LOG_LEVEL is hard-coded to WARNING. This should be under a GUC's control. This commit unified ic-proxy log level under GUC gp_log_interconnect's control, and remove macro IC_PROXY_LOG_LEVEL.
…rol, and remove macro IC_PROXY_LOG_LEVEL" (#14207) This reverts commit 24140214f21df4276f239b061754eefe610c1d8a. This change caused ICW proxy pipelines to go red [1][2][3][4][5][6] due to extra logging. Reverting so that pipelines go back to green again. [1] https://prod.ci.gpdb.pivotal.io/teams/main/pipelines/gpdb_master/jobs/icw_gporca_icproxy_rhel8/builds/568 [2] https://prod.ci.gpdb.pivotal.io/teams/main/pipelines/gpdb_master/jobs/icw_planner_icproxy_rhel8/builds/571 [3] https://prod.ci.gpdb.pivotal.io/teams/main/pipelines/gpdb_master/jobs/icw_gporca_icproxy_ubuntu18.04/builds/1324 [4] https://prod.ci.gpdb.pivotal.io/teams/main/pipelines/gpdb_master/jobs/icw_planner_icproxy_ubuntu18.04/builds/1322 [5] https://prod.ci.gpdb.pivotal.io/teams/main/pipelines/gpdb_master/jobs/icw_gporca_icproxy_centos7/builds/1342 [6] https://prod.ci.gpdb.pivotal.io/teams/main/pipelines/gpdb_master/jobs/icw_planner_icproxy_centos7/builds/1344
This test case has been flaky for a while, and while it's being worked on and discussed, it currently makes developers either ignore red tests or blocks other work from going in. For now, add a FIXME and disable the test case for Orca. Discussion: https://groups.google.com/a/greenplum.org/g/gpdb-dev/c/u3-D7isdvmM
If a DynamicBitmapScan was under a NLJ, memory would increase very quickly and the system would soon terminate due to OOM. Additionally, performance was slower than an equivalent plan using regular Bitmap Scans. This only affected Orca as only Orca uses Dynamic Scans. This change had been done for regular index scans, but was not done for bitmap scans.
This is a follow-up to 24140214f21, which was reverted for being too verbose by default, causing CI failures. This reintroduces the gp_log_interconnect GUC to ic-proxy logging and follows the specifications outlined in cdbvars.c. The ic_proxy_log macro is replaced with elog and elogif for WARNINGs/ERRORs and other events respectively. The scheme used: TERSE[LOG]: Server main loop events, signals VERBOSE[LOG]: Startup, (un)register, close/shutdown, EOF receipt etc. DEBUG[DEBUG1]: Special packet (BYE/HELLO) transfer, pause/resume events DEBUG[DEBUG3]: Less interesting packet transfers, caching DEBUG[DEBUG5]: Very low-level tracing info
This commit amends commit d63731d. Fallback needs to happen inside the rollback'd transaction as that is where the stale cache entry is not cleaned up properly.
The external tables have distribution policy but it does not dictate actual data distribution. It is only used when unloading data, to compare with source table's distribution policy. Therefore, when supporting ALTER TABLE SET DISTRIBUTED BY for external tables, we don't really need to re-organize the table but just need to make sure the catalog change happens.
avamingli
dismissed stale reviews from gfphoenix78 and my-ship-it
via
561879f
avamingli
requested review from
my-ship-it,
gfphoenix78,
yjhjstz and
jiaqizho
and removed request for
my-ship-it
12 tasks
jiaqizho
approved these changes
Dec 30, 2024
yjhjstz
approved these changes
Dec 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #ISSUE_Number
What does this PR do?
Type of Change
Breaking Changes
Test Plan
make installcheckmake -C src/test installcheck-cbdb-parallelImpact
Performance:
User-facing changes:
Dependencies:
Checklist
Additional Context
CI Skip Instructions