Merged
Conversation
When update or delete statement errors out because of the CTID is not belong to the local segment, we should also print out the CTID of the tuple so that it will be much easier to locate the wrong- distributed data via: `select * from t where gp_segment_id = xxx and ctid='(aaa,bbb)'`.
We've seen a panic case on gpdb 6 with stack as below, 3 markDirty (isXmin=0 '\000', tuple=0x7effe221b3c0, relation=0x0, buffer=16058) at tqual.c:105 4 SetHintBits (xid=<optimized out>, infomask=1024, rel=0x0, buffer=16058, tuple=0x7effe221b3c0) at tqual.c:199 5 HeapTupleSatisfiesMVCC (relation=0x0, htup=<optimized out>, snapshot=0x15f0dc0 <CatalogSnapshotData>, buffer=16058) at tqual.c:1200 6 0x00000000007080a8 in systable_recheck_tuple (sysscan=sysscan@entry=0x2e85940, tup=tup@entry=0x2e859e0) at genam.c:462 7 0x000000000078753b in findDependentObjects (object=0x2e856e0, flags=<optimized out>, stack=0x0, targetObjects=0x2e85b40, pendingObjects=0x2e856b0, depRel=0x7fff2608adc8) at dependency.c:793 8 0x00000000007883c7 in performMultipleDeletions (objects=objects@entry=0x2e856b0, behavior=DROP_RESTRICT, flags=flags@entry=0) at dependency.c:363 9 0x0000000000870b61 in RemoveRelations (drop=drop@entry=0x2e85000) at tablecmds.c:1313 10 0x0000000000a85e48 in ExecDropStmt (stmt=stmt@entry=0x2e85000, isTopLevel=isTopLevel@entry=0 '\000') at utility.c:1765 11 0x0000000000a87d03 in ProcessUtilitySlow (parsetree=parsetree@entry=0x2e85000, The reason is that we pass a NULL relation to the visibility check code, which might use the relation variable to determine if hint bit should be set or not. Let's pass the correct relation variable even it might not be used finally. I'm not able to reproduce the issue locally so I can not provide a test case but that is surely a potential issue. Reviewed-by: Ashwin Agrawal <aashwin@vmware.com> (cherry picked from commit 8581169)
ic_proxy_backend.h includes libuv's uv.h header, and ic_proxy_backend.h was being included in ic_tcp.c, even when compiling with --disable-ic-proxy.
For a table partitioned by timestamp column, a query such as
SELECT * FROM my_table WHERE ts::date == '2020-05-10'
should only scan on a few partitions.
ORCA previously supported only implicit casts for partition selection.
This commit, extends ORCA to also support a subset of lossy (assignment)
casts that are order-preserving (increasing) functions. This will
improve ORCA's ability to partition elimination to produce faster plans.
To ensure correctness, the additional supported functions are captured
in an allow-list in gpdb::IsFuncAllowedForPartitionSelection(), which
includes some in-built lossy casts such as ts::date, float::int etc.
Details:
- For list partitions, we compare our predicate with each distinct
value in the list to determine if the partition has to be
selected/eliminated. Hence, none of the operators need to be changed
for list partition selection
- For range partition selection, we check bounds of each partition and
compare it with the predicates to determine if partition has to be
selected/eliminated.
A partition such as [1, 2) shouldn't be selected for float = 2.0, but
should be selected for float::int = 2. We change the logic for handling
equality predicates differently when lossy casts are present (ub: upper
bound, lb: lower bound)
if (lossy cast on partition col):
(lb::int <= 2) and (ub::int >= 2)
else:
((lb <= 2 and inclusive lb) or (lb < 2))
and
((ub >= 2 and inclusive ub ) or (ub > 2))
- CMDFunctionGPDB now captures whether or not a cast is a lossy cast
supported by ORCA for partition selection. This is then used in
Expr2DXL translation to identify how partitions should be selected.
Fault injection is expected to be *very* cheap, we even enable it on production builds. That's why I was very surprised when I saw 'perf' report that FaultInjector_InjectFaultIfSet() was consuming about 10% of CPU time in a performance test I was running on my laptop. I tracked it to the FaultInjector_InjectFaultIfSet() call in standard_ExecutorRun(). It gets called for every tuple between 10000 and 1000000, on every segment. Why is FaultInjector_InjectFaultIfSet() so expensive? It has a quick exit in it, when no faults have been activated, but before reaching the quick exit it calls strlen() on the arguments. That's not cheap. And the function call isn't completely negligible on hot code paths, either. To fix, turn FaultInjector_InjectFaultIfSet() into a macro that's only few instructions long in the fast path. That should be cheap enough. Reviewed-by: Ashwin Agrawal <aashwin@vmware.com> Reviewed-by: Jesse Zhang <jzhang@pivotal.io> Reviewed-by: Asim R P <pasim@vmware.com>
In grouping set cases, we need to replace our private subplan into plan tree. But for subplan's RelOptInfo will not create if the subplan does not have any relaton in its fromlist. Check pointer in simple_rte_array and simple_rel_array to promise which is not null before replacement.
The Greenplumn release builds are configured without any extra configure flags, it only use the ones specified in gpAux/Makefile, so we have to enable ic-proxy in this file to include the ic-proxy feature in relase builds. Reviewed-by: Shaoqi Bai <bshaoqi@vmware.com> (cherry picked from commit 4897875)
The URL stored in the 'filename' field of the error log was stored
incorrectly:
postgres=# SELECT filename FROM gp_read_error_log('sreh_ext_err_tbl');
filename
------------------------------------------------------------------------------------
(null) [/home/heikki/git-sandbox-gpdb/master/src/test/regress/data/bad_data1.data]
(null) [/home/heikki/git-sandbox-gpdb/master/src/test/regress/data/bad_data1.data]
...
On 5X_STABLE, the URL is stored in place of the '(null)'. This got broken
in 6X_STABLE. The root cause is that we no longer keep the 'url' in the
CopyState->filename field for gpfdist external tables.
Fixes https://github.com/greenplum-db/gpdb/issues/10542
Reviewed-by: (Jerome)Junfeng Yang <jeyang@pivotal.io>
Currently FDW expects that foreign plan's fdw_private field contains a private list of DefElem nodes (options) that should not be CDB mutated. We don't try to mutate it in master but still mutate in 6X that causes "ERROR: unrecognized node type: 920". Backport described behaviour from master. Steps to reproduce: create extension file_fdw; create server file foreign data wrapper file_fdw; create foreign table t_file (a int) server file options( mpp_execute 'all segments', filename '/tmp/1_<SEGID>.csv', format 'csv'); \! echo '0' > /tmp/1_0.csv \! echo '1' > /tmp/1_1.csv \! echo '2' > /tmp/1_2.csv select count(*) from t_file; ERROR: unrecognized node type: 920 (nodeFuncs.c:2932)
Quickfix to move the 6X_STABLE pipeline. virtualenv recently started installing six by itself, which meant that it was not installed as a subdependency into /tmp/py-requirements. We need it there so that we can copy it into the vendored stack, so tell pip to --ignore-installed.
…ent loop (#10610) After losing tcp connection between gpmmon and gpsmon and subsequent its reestablishment the gpsmon enters to infinite loop inside libevent routine of timeout event processing. The reason was in calling event_set function under new connection establishment on currently working (not released) tcp_event that rewrites internal flags of that event and puts it into unconsistent state. The current fix releases tcp_event from participating in event loop (takes to unpending state) under connection closing from gpsmon by timeout so that the next new connection have to set tpc_event in event loop again.
Add gpreload behave test * gpreload test where a table (public.t1) and view (gpreload_schema.t1) have the same name. Add gpreload CI test jobs. Co-authored-by: Ed Espino <eespino@vmware.com>
optarg is a global and is not required to be passed to process_greenplum_option. Also, for bool parameters pg_strdup must not be called else it will fail with cannot duplicate null pointer (internal error)
Unlogged tables do not propagate to replica servers, skip them and their initialization forks.
1. SQL script in /gpcontrib/gp_internal_tools/gp_internal_tools--1.0.0.sql and /gpcontrib/uninstall_gp_session_state.sql changes current search_path, this commit set search_path to default at the end of the above two SQL scripts 2.fix typos in /gpcontrib/gp_internal_tools/gp_internal_tools--1.0.0.sql 3.fix the expect caused by this fix. This fixes the issue: https://github.com/greenplum-db/gpdb/issues/10575. (cherry picked from commit 9d9662f) Co-authored-by: LittleWuCoding <31984989+LittleWuCoding@users.noreply.github.com>
Previously, when backends connect to a proxy, we need to setup domain socket pipe and send HELLO message(recv ack message) in a blocking and non-parallel way. This makes ICPROXY hard to introduce check_for_interrupt during backend registeration. By utilizing libuv loop, we could register backend in paralle. Note that this is one of the step to replace all the ic_tcp backend logic reused by ic_proxy currently. In future, we should use libuv to replace all the backend logic, from registeration to send/recv data. Co-authored-by: Ning Yu <nyu@pivotal.io> (cherry picked from commit 608514c)
ic-proxy is developed with libuv, the minimal supported libuv version is 1.18.0. But in commit 608514, we introduce new API in libuv 1.19, which break compatibility on os like Ubuntu 18.04 whose default libuv version is 1.18. We should keep our code base align with libuv 1.18, and replace the new API in libuv 1.19. The API change is mainly about how to access data field in uv handle and uv loop. The new API uses function interface like `uv_handle_set_data` and `uv_handle_get_data` to access data filed, while the old API in 1.18 access the data filed directly. Note that in the latest libuv version 1.38.2, the old API and new API are both supported. And libuv is stable enough to support the old API for a long time. (cherry picked from commit ab36eb9)
ic-proxy supports to handle cancel signal during backend connection setup now, so external_table test case could be enabled. Reviewed-by: Ning Yu <nyu@pivotal.io> (cherry picked from commit 1789e43)
* docs - clarify gpcopy supported greenplum versions * use or later rather than +
…… (#10635) * docs - add information on table rewrites when executing ALTER TABLE commands. Information is in Notes section of ALTER TABLE. Also, made some minor edits. * docs - updates based on review comments. -simplified table listing ALTER TABLE commands that perform table rewrites. -add links to table rewrite information.
There is a race condition between receiveFrom() and dbms_pipe_session_A::createImplicitPipe(). Sleep an extra seconds is not reliable. So wait until the data is available. This PR changes the semantics of receiveFrom(), i.e. wait until the data is available, no timeout happens. Reviewed-by: Hubert Zhang <hzhang@pivotal.io> (cherry picked from commit 2f170b3)
This creates an explain pipeline which is used to run query workloads using explain and identify plan differences during feature development. This previously existed on 5X, but with the Orca in GPDB effort the architecture needed to be updated. The new pipeline has the following features for use with slack commands: - Comparing a SHA with a baseline SHA - Identifying and outputting plan diffs - Summarizing the results with planning time, any fallbacks/crashes/timeouts, and the type of diff (cost, cardinality, plan change) - Enabling custom gucs (turning optimizer off, using a different cost model, enabling/disabling a feature flag/join order) Some use cases that we've come across: - Checking whether a new join order algorithm influences optimization time/dramatically changes plans One of the main goals with this change is to make it easy to add workloads. The slack command can be used in the following format: \test_gpdb6_explain <SHA_remote> <SHA> <baseline_SHA_remote> <baseline_SHA> <workload> [optional_gucs] where workloads is either "all" or a list with no spaces of the names of the workloads to run (eg: workload1,workload2,workload3). and gucs, if specified, are in the format: ``` optimizer=off enable_nestloop=on ```
This is kept as a separate commit as an example of how to add additional workloads. Co-authored-by: Chris Hajas <chajas@vmware.com>
Raise a meaningful error message for this case. GPDB doesn't support alter type on primary key and unique constraint column. Because it requires to drop - recreate logic. The drop currently only performs on master which lead error when recreating index (since recreate index will dispatch to segments and there still an old constraint index exists). This fixes the issue https://github.com/greenplum-db/gpdb/issues/10561. Reviewed-by: Hubert Zhang <hzhang@pivotal.io> (cherry picked from commit 32446a3)
* allow NEq predicates with lossy casts for PS * Updating some mdp files that were missing the PartitionTypes XML tag * Update partition_pruning result file for planner * adding new testcase for NEq * move icg tests from partition_pruning to gporca * add test for checking multilevel range/list PS Co-authored-by: Hans Zeller <hzeller@vmware.com>
Commit 445fc7c hardened some parts of analyzedb. However, it missed a couple of cases. 1) When the statement to get the modcount from the pg_aoseg table failed due to a dropped table, the transaction was also terminated. This caused further modcount queries to fail and while those tables were analyzed, it would error and not properly record the mod count. Therefore, we now restart the transaction when it errors. 2) If the table is dropped and then recreated while analyzedb is running (or some other mechanism that results in the table being successfully analyzed, but the pg_aoseg table did not exist during the initial check), the logic to update the modcount may fail. Now, we skip the update for the table if this occurs. In this case, the modcount would not be recorded and the next analyzedb run will consider the table modified (or dirty) and re-analyze it, which is the desired behavior.
* docs - add information on upgrading to PostGIS 2.5.4 Upgrade instructions 2.1.5 to different versions of 2.5.4 * docs - upgrade to PostGIS 2.5.4 review comments * docs - more review comment updates. reorder upgrade sections. clarify removing PostGIS package, is for removing the gppkg * docs - minor edit * docs - review updates - more emphasis on removing PostGIS from a database deleting objects. -Create separate paragraph in Upgrading section. -Add warning in Removing PostGIS section * docs - minor review comment update * small edits Co-authored-by: David Yozie <dyozie@pivotal.io>
When we convert semi join to inner join, a distinct agg on ctid is added above the hash join node. But tuples from function scan have no ctid. So the assert check failed. Set a synthetic ctid based on a fake ctid by call existed funciton slot_set_ctid_from_fake. (cherry picked from commit d8886cf)
When converting semi-join to inner-join, a distinct agg on ctid is added above the hash-join node. But the fake ctids generated in Values Scan were invalid, with offset number 0, which caused an assertion failure. This patch is based on commit d8886cf, which fixed the same issue for Function Scans. Co-authored-by: dh-cloud <60729713+dh-cloud@users.noreply.github.com> Co-authored-by: Jesse Zhang <sbjesse@gmail.com>
MASTER_DATA_DIECTORY must be exported to before calling gpconfig else it will fail to set the GUC Allow setting DCA_VERSION_FILE to enable testing Allow overriding the value of DCA_VERSION_FILE so that during testing it can be manipulated. Also add a test for the same to ensure that DCA configuration GUCs are set properly on the environment.
When calculating safeChunksThreshold of runaway in resource group, we used to divide by 100 to get the number of safe chunks. This may lead to small chunk numbers to be rounded to zero. Fix it by storing safeChunksThreshold100(100 times bigger than the real safe chunk) and do the computation on the fly. Reviewed-by: Ning Yu <nyu@pivotal.io> (cherry picked from commit 757184f)
…10679)
This approach special cases gp_segment_id enough to include the column
as a distributed column constraint. It also updates direct dispatch info
to be aware of gp_segment_id which represents the raw value of the
segment where the data resides. This is different than other columns
which hash the datum value to decide where the data resides.
After this change the following DDL shows Gather Motion from 2 segments
on a 3 segment demo cluster.
```
CREATE TABLE t(a int, b int) DISTRIBUTED BY (a);
EXPLAIN SELECT gp_segment_id, * FROM t WHERE gp_segment_id=1 or gp_segment_id=2;
QUERY PLAN
-------------------------------------------------------------------------------
Gather Motion 2:1 (slice1; segments: 2) (cost=0.00..431.00 rows=1 width=12)
-> Seq Scan on t (cost=0.00..431.00 rows=1 width=12)
Filter: ((gp_segment_id = 1) OR (gp_segment_id = 2))
Optimizer: Pivotal Optimizer (GPORCA)
(4 rows)
```
(cherry picked from commit 10e2b2d)
Proxy bgworker will become orphan process after postmaster is dead due to the lack of checking pipe postmaster_alive_fds[POSTMASTER_FD_WATCH]. Epoll this pipe inside proxy bgworker main loop as well. Reviewed-by: Ning Yu <nyu@pivotal.io> (cherry picked from commit 9ce59d1)
Resource group used to access resGroupSlot in SessionState without lock. This is correct when session only access resGroupSlot by itself. But as we introduced runaway feature, we need to traverse the current session array to find the top consumer session when redzone is reached. This requires: 1. runaway detector should hold shared resgroup lock to avoid resGroupSlot is detached from a session concurrently when redzone is reached. 2. normal session should hold exclusive lock when modifying resGroupSlot in SessionState. Also fix a compile warning. Reviewed-by: Ning Yu <nyu@pivotal.io> (cherry picked from commit a4cb06b)
…sue (#10745) fix match column condition to resovle primary key conflict when using the gpload merge mode to import data to the Multi-level partition table fix fail when special char and capital letters in column names Co-authored-by: XiaoxiaoHe <hxiaoxiao@vmware.com>
This will make LISTEN and NOTIFY work on the QD node.
When doing 'VACUUM FULL', 'swap_relation_files' updates the pg_class entry but not increase the command counter, so the later 'vac_update_relstats' will inplace update the 'relfrozenxid' and 'relhasindex' of the old tuple, when the transaction is interrupted and aborted on the QE after this, the old entry is corrupted.
The GROUPING SETS statement with multiple canonical rollups working on randomly distributed partitioned table causes rebuilding of root->simple_rte_array and root->simple_rel_array on planning stage by PostgreSQL optimizer. The rebuilding process initializes both RTEs and RelOptInfos item by item and doesn't take into account that the build_simple_rel() routine is recursive for inherited relations and requires a full list of already initialized child RTEs inside root->simple_rte_array. As a result, the call of build_simple_rel() fails on the access to child RTEs. The current fix segregates loops of building root->simple_rte_array and root->simple_rel_array trying to leave a whole semantics unchanged.
…0751) * fix gpload fial when capital letters in column name in merge mode add double quotations for column names when create staging table omit distributio Co-authored-by: XiaoxiaoHe <hxiaoxiao@vmware.com>
…mode (#10751)" (#10778) This reverts commit 4ba1190.
leskin-in
approved these changes
Sep 17, 2020
leskin-in
left a comment
leskin-in
left a comment
There was a problem hiding this comment.
Note:
- 6a6da69 forbids to build
postgres_fdwon MacOS. @darthunix - 4d248ac closes ADBDEV-103. We may need to conduct a test of the case described there and add a confirmation to JIRA.
Collaborator
@leskin-in We already merged this fix in 6X and 5X. What do you mean?
Yes, of course. There are several fixes by Maksim in this Pivotal release. @maksm90, the main purpose of these reviews is the detection of upstream changes that may break our fork. |
Stolb27
approved these changes
Sep 18, 2020
|
@Stolb27 although they were merged earlier, I suppose we could conduct the tests again, as we are re-importing the change from upstream. In the current case this is likely of little (if any) value, but I suppose this is a good practice we should follow, as not all changes re-imported are exactly the same as the ones made to our fork. |
maksm90
approved these changes
Sep 21, 2020
RekGRpth
pushed a commit
that referenced
this pull request
Dec 3, 2025
## ADBDEV-8787: (v6) Run tests for feature branches with slashes (#115) - Update pull-request branches to '**'
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
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.
No description provided.