forked from yugabyte/yugabyte-db
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Test fixes #65
Open
arpang
wants to merge
37
commits into
pg15
Choose a base branch
from
pg15-test
base: pg15
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Test fixes #65
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
Summary: Merge with PG13 Test Plan: None Suppressed compiling Postgres's extension. Removed obsolete Postgres's types from docdb
…hecking the condition for creating a primary key index Summary: The attribute `yb_is_add_primary_key` was created as part of the `AlterTableCmd` struct to indicate this alter command is for adding primary key. In the past, there were 2 subcommands involved when creating a primary key: 1) `AddIndex` 2) `SetNotNull` (only if any one of the primary key attributes are not set as NOT NULL). In PG13, setting the primary key columns to NOT NULL is triggered in a separate API called `transformIndexConstraint`. The boolean flag has been set in the corresponding section where `AlterTableCmd` is prepared to alter the primary key column to be not null. Here is the excerpt from the PG13 comment mentioning the need to satisfy NOT NULL requirement prior to creating primary key index: ```Now we expect that the parser inserted any required ALTER TABLE SET NOT NULL operations before trying to create a primary-key index.``` Test Plan: Jenkins Reviewers: neil, myang Reviewed By: myang Subscribers: yql Differential Revision: https://phabricator.dev.yugabyte.com/D21331
Summary: Fix two compilation errors: - release build got -Wsometimes-uninitialized compilation error because of Assert(false) that is treated differently in release/debug builds - gcc build got -Werror=comment because of bad comment Test Plan: Jenkins: compile only, rebase: pg13 ./yb_build.sh fastdebug --gcc11 --no-initdb ./yb_build.sh release --clang15 --no-initdb Reviewers: smishra, neil Reviewed By: neil Subscribers: yql Differential Revision: https://phabricator.dev.yugabyte.com/D22577
Summary: YbClearCurrentTransactionId() was introduced to handle temp tables created during concurrent materialized view refreshes correctly. PG13 no longer uses `CurrentTransactionState->transactionId` and `MyPgXact`. This diff changes the code to use the PG13 equivalent of these fields/structures - `CurrentTransactionState->fullTransactionId` and `MyProc->xid`. Test Plan: Jenkins: skip Reviewers: neil, smishra Reviewed By: neil, smishra Subscribers: yql Differential Revision: https://phabricator.dev.yugabyte.com/D21721
Summary: When migrating from PG 11.2 to PG 15, we come across commit 49fa99e54ec0ded180b52a4a14e543702d53e8c9 that makes make_greater_string private: Move pattern selectivity code from selfuncs.c to like_support.c. ... This change localizes the use of pattern_fixed_prefix() and make_greater_string() so that they no longer need be exported. (We might get pushback from extensions about that, perhaps, in which case I'd be inclined to re-export them in a new header file like_support.h.) ... ybgin uses make_greater_string for tsvector prefix logic. Since it is not trivial to indirectly get the benefit of make_greater_string through the publicly exposed support functions (they take complex Node argument), lean towards re-exposing the function through new header yb_like_support.h and new function yb_make_greater_string. If, in the future, upstream postgres decides to expose make_greater_string, these changes can be scrapped in favor of using make_greater_string again. Test Plan: Jenkins: compile only, rebase: pg15 Reviewers: neil, amartsinchyk Reviewed By: amartsinchyk Subscribers: yql Differential Revision: https://phabricator.dev.yugabyte.com/D23110
Summary: build-support/jenkins/build-and-test.sh involves running doctest on select python files. This is what jenkins runs, whereas developers typically run ./yb_build.sh. doctest appears to have a strange bug where it does not like it when sys.stdout is set to something else by the time of the end of file. This doesn't seem to matter when executing the python script itself: it only matters for doctest. PG 15 merge involves updating generate_unaccent_rules.py, and this update hits the above bug. Appease doctest by restoring sys.stdout to the original object at the end of the file. It seems wasteful to run doctest on files that don't have docstrings, but that can be fixed in the future. Test Plan: jenkins: compile only, rebase: pg15 Reviewers: mbautin, neil, steve.varnau Reviewed By: steve.varnau Subscribers: yql Differential Revision: https://phabricator.dev.yugabyte.com/D23266
Summary: In the master branch, for memory tracking, we added `oldsize` for the block. It overwrites the variable with the same name in the outer scope, defined for the chunk. However, in PG upstream, the `oldsize` in the outer scope is referenced in the inner scope. Because of the naming overwritting, after merging, the reference used becomes incorrect. **Solution** I basically "backport" how the upstream creates the variable for "block old size" and use it is for YB memory tracking. Along with this, it also fixes two potential overflow issues in the existing code. The fixes are needed in the master as well. So a backport to the PG13 branch will be committed later in a separate diff. Original commit: 83c6b6a/D21128 Test Plan: Jenkins: rebase: pg15 Reviewers: neil, mbautin, telgersma Reviewed By: telgersma Subscribers: smishra, yql Differential Revision: https://phabricator.dev.yugabyte.com/D24000
Fixed compilation errors in /access, /bootstrap, /catalog, and /commands Fixed compilation errors in /parser Fixed compilation errors in /commands Fixed compilation errors in /executor Fixed compilation errors in /libpq Fixed compilation errors in /optimizer Fixed compilation errors in /utils Completed fixing errors for directories in /backend/* Commented out Yugabyte's code for tools in src/bin - Revisit later Add missing files in contrib Add missing files in postgres/doc
Fixed compilation errors in src/backend Fixed compilation errors in pg_dump Used Pg15 structure for FunctionCallInfo Remove Yugabyte version of pull_varattnos_walker() to avoid duplicating code with Pg15. Activate Yugabyte cluster-connecting code in InitPostgres
Summary: Adds missing oid columns and indexes to YB system tables Test Plan: N/A Reviewers: mihnea, neil Reviewed By: neil Subscribers: yql Differential Revision: https://phabricator.dev.yugabyte.com/D24959
Summary: Initdb bootstrap step working Test Plan: Jenkins: skip Reviewers: mihnea, neil Reviewed By: neil Subscribers: yql Differential Revision: https://phabricator.dev.yugabyte.com/D25090
Summary: Initdb fixes till setup_collation() Test Plan: Jenkins: skip Reviewers: neil, mihnea Reviewed By: neil Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D25341
Summary: With this diff, initdb is working end to end except for pg_stat_statements. Test Plan: Jenkins: rebase: no Reviewers: neil, mihnea Reviewed By: neil Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D25787
Summary: This diff addresses the below issues/fixes: # Uncomments python doctest failures fix: https://phorge.dev.yugabyte.com/D23266 # PG changed default value of pg_class.reltuples from 0 to -1: postgres/postgres@3d351d9 # Fixes index scan # IndexQualInfo is removed and IndexClause is introduced # ModifyTablePath has subpath (a single source data path) instead of a list # Fixes insert queries which did not have values for all the columns. # Build failure cause due to missing arg in InstrAlloc # Create table failure FailedAssertion("ptr == NULL || nodeTag(ptr) == type") # Single row update optimization (yb_single_row_update_or_delete_path) Consider table: create table emp(id int primary key, name text). Queries which were not working/working incorrectly before this diff that has been fixed: # insert into emp values (1), (2), (3); # select * from emp where id = 1; (this was working but was taking foreign scan path and not index scan) # delete from emp where id = 1; # delete from emp where id >= 1; # update emp set name = 'John' where id = 1; # update emp set name = 'John' where id >= 1; # update my_emp set id = id + 4 where id = 1; (point PK update) # update my_emp set id = id + 4 where id > 1; (range PK update) # Tested updates with before update triggers # Tested updates with pushdown (took test from yb_dml_single_row.sql) Test Plan: Jenkins: rebase: no Reviewers: tnayak, smishra, neil Reviewed By: tnayak Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D27692
Test Plan: Jenkins: rebase: no ./yb_build.sh --java-test org.yb.pgsql.TestPg15Regress#testPg15Regress Reviewers: smishra, neil Reviewed By: smishra Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D27769
Summary: Summary of the changes: # PG15 commutes `RestrictInfo.clause` if the variable is on the right (for instance, in query `select * from emp where 1 = id;`). For implementation, search `commrinfo = commute_restrictinfo(rinfo, comm_op);` in `indxpath.c`. As a consequence function `deconstruct_indexquals` and struct `IndexQualInfo` have been removed. This diff handles this change for YB. # Instead of maintaining `is_hashed` state (`IndexQualInfo` is removed), it is converted into a helper function. See `is_hashed` in `yb_scan.c`. This is open for review. # Handles type change of `ReservoirStateData.randstate` from array to a struct of two `uint64`s. Test Plan: Jenkins: rebase: no Reviewers: smishra, mihnea, neil, tnayak Reviewed By: tnayak Subscribers: yql, ybase Differential Revision: https://phorge.dev.yugabyte.com/D27737
Summary: This diff fixes the following: # Enables the code block in `ClientAuthentication` in `auth.c`, which was disabled during the merge. A minor code modification is done related to the oid now being a regular column. # Enables code blocks in `hba.c` which were disabled during the merge. Previously, a portion of `tokenize_file` was refactored out into a common helper function `tokenize_line` for PG/YB. Subsequent changes in PG upstream led to conflicts during the merge. This diff retains the PG upstream code as it is, along with a YB-specific function `yb_tokenize_line`. It also renames `tokenize_hardcoded` to `yb_tokenize_hardcoded` as it YB-specific. # Adds missing case for `T_BackfillIndexStmt` in the function `ClassifyUtilityCommandAsReadOnly.` # Removes ` Assert(0);` from function `IndexBackfillHeapRangeScan`. YB_TODO comment related to handling the `progress` parameter is already present in the function; hence, assert(0) is not required. With fixes #2, #3, and #4, `CREATE INDEX CONCURRENTLY` works on pg15 branch. Test Plan: Jenkins: rebase: no Reviewers: smishra, neil, jason Reviewed By: jason Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D27860
Summary: This diff fixes the optimizer and executor directory. # In pg15, oid column is treated like a regular column, hence made `ybFetchNext` return virtual tuple table slot always (see function comment for more context) # `systable_recheck_tuple` doesn't work in YB. Previously, we disabled calling it in YB. In the newer PG version, new calls to this function were introduced and that caused issues. This can happen in the future versions as well. Hence, I refactored the function itself to return without executing it in YB. # PG introduced support for collation in the hashing functions (postgres/postgres@5e1963f). This diff disables it for YB (this is open for review). # Enabled the code blocks disabled during the merge related to YB batched nested loop join. # Removes unused OID returned from YB modify functions (insert, update). # Fixes related to single row modify. Test Plan: Jenkins: rebase: no Reviewers: neil, tnayak Reviewed By: tnayak Subscribers: smishra, yql Differential Revision: https://phorge.dev.yugabyte.com/D27766
Summary: This diff fixes the todos in catalog directory: # Removes `Assert(!shared_relation);` from `heap_create_with_catalog` function as pg15 creates entries in pg_type for array types of shared relations as well. # Cleans up functions that have been removed from upstream pg: `GetNewOid`, `RemoveCollationById`, `RemoveConversionById`, `isSharedObjectPinned`. # Discussion related to `pg_stat_progress_create_index ` changes in `system_views.sql`: https://yugabyte.slack.com/archives/C03L0TLANHY/p1689070967517019 # Changes related to pg15 upstream replacing explicit object pinning with range test: postgres/postgres@a49d081 (One more pass is required to handle this change for YB. I will do it as part of a subsequent diff. In this diff, I am only cleaning up YB-specific function calls, which are anyway deprecated / unusable and hence were disabled during merge). Test Plan: Jenkins: rebase: no Reviewers: neil, yguan, fizaa Reviewed By: yguan Subscribers: mihnea, yql Differential Revision: https://phorge.dev.yugabyte.com/D27741
Summary: This diff fixes the following issues in the utils directory: # Adds missing YB functions in pgstat.c. # Removes special handling of oid in relcache.c. # Removes function `assign_pgstat_temp_directory` from guc.c, which was removed from PG upstream. # PG has introduced an optimization to use generation to allocate memory context. This was causing merge join to fail in YB. Disabled it for now. Test Plan: Jenkins: rebase: no Reviewers: smishra, neil, tvesely Reviewed By: tvesely Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D27985
Summary: Fix clangd indexing if the DLSUFFIX definition does not appear in compiler commands. Test Plan: Jenkins: skip ./yb_build.sh --cio bin/run_codecheck Reviewers: tfoucher Reviewed By: tfoucher Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D28478
Summary: This diff fixes the following runtime issues. Except for one (highlighted below), these issues have existed since the initial merge of YB with PG15 changes: in tablecmds.c # Modifies macro `YBGetNotNullConstraintAttr` to read `oid` from the `Form_pg_constraint` as `HeapTupleGetOid` is unavailable in PG15. # PG (in [[ postgres/postgres@e1551f9 | commit ]] ) replaced `AttrNumber *` with `AttrMap *` and `convert_tuples_by_name_map` with `build_attrmap_by_name`. To compile the code after the YB and PG15 merge, YB-specific functions/code block using `AttrNumber *` and `convert_tuples_by_name_map` were disabled. This diff fixes it. # During the merge commit ([[ yugabyte@a6974e5#diff-551494d846d56417f70e5a060c2a8f90ba65c43019ab7bfdc2c84b109c83f947 | Merge with some of Postgres 15 ]]), `YbHeapTupleGetOid` and `YB_HACK_INVALID_OID` were introduced as hacks to compile the pg15 branch (both of these are `-1`). Some of these usages are removed/replaced with correct OIDs in this diff. # PG (in [[ postgres/postgres@5815696 | commit ]] ) replaced function `addRTEtoQuery` with `addNSItemToQuery`. To compile the code after the YB and PG15 merge, YB-specific function calls to `addRTEtoQuery` were disabled. This diff fixes this. # Modifies function `YbATCopyPolicyObjects` to generate new oid while cloning policies using `GetNewOidWithIndex` as is done in `CreatePolicy`. in indexcmds.c # Re-merged the `DefineIndex` function. This function is not merged properly since the rebase commit [[ yugabyte@06d9200 | Rebase with yb::master up to commit 61302b0 ]]. in dbcommands.c # Fixes cache reference leak on db creation. This was a bug introduced during the initial merge commit ([[ yugabyte@7e541a1#diff-e825bdb435bd1205f1cdb2db105299d8bafd7c0a8f8e4d7bd7790ca87deff582 | Complete merging with Postgres 15.2 ]]). checkpointer.c: # `DROP DATABASE` was broken at `RequestCheckpoint` function call. I am unaware when did it start breaking. I am also unaware why it doesn't fail on YB master branch. The function (RequestCheckpoint) is not applicable in YB because it doesn't use PG WAL. Hence, disabled this function for YB. in tablegroup.c # Removes a done YB_TODO comment. This was a verification comment. I verified that the values are populated correctly and required no code changes. in reloptions.c # PG (in [[ postgres/postgres@1bbd608 | commit ]]) introduced separate handling of reloptions for partitioned tables via function `partitioned_table_reloptions`. This change in YB was causing partitioned table creation to fail with `FailedAssertion("numoptions <= num_relopt_elems")`. This was because in PG, there no options for partitoned tables but in YB, there is `colocation_id` option. The diff handles it. in nodeModifyTable.c: # Fixes `relcache reference leak` on insertion into a table with secondary indexes by not re-opening already open indices. This issue was introduced during initial YB and PG15 merge commit ([[ yugabyte@a6974e5#diff-7298d5c117fa9b5b7b8642512d380ab43b432a2d2216d4f2ef23096dfac0c921 | "Merge with some of Postgres 15" ]]). # PG in [[ postgres/postgres@25e777c#diff-08af8c53c4fe685f813673454d48447222d490bccd75b0c88f2f6a9b83f66fab | commit ]] split ExecUpdate and ExecDelete. This commit was introduced to YB as commit [[ 7e541a1 | Complete merging with Postgres 15.2 ]]. Consequently, this introduced a bug where secondary index entries were not deleted for DELETE statements. This diff fixes it. ri_triggers.c: # [[ yugabyte@3312964#diff-9d0e6cee391e5362dc643eecdac580f7d2d0c6c378ad0ff1b35bf4592c388b2f | Utils fixes ]] commit introduced a regression by freeing up `HeapTuple` `new_row` too soon causing FK violation error on insertion even when there are no violations. The diff fixes the bug. in matview.c # frees up `HeapTuple tuple`, if `shouldFree` is set to true by `ExecFetchSlotHeapTuple`. Test Plan: Jenkins: skip, rebase: pg15 Reviewers: jason Reviewed By: jason Subscribers: fizaa, mihnea, smishra, dmitry, ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D28432
Summary: This diff fixes the following: # Cleans up (disabled) blocks of code no longer applicable pg15. # Renames `NEIL_OID` and `NEIL_TODO` to `YB_TODO` for better tracking. # `ObjectIdAttributeNumber` is not available in upstream PG15 as OID is a regular column henceforth. It is retained in YB's pg15 branch as a hack to compile the code till all the references to it are removed. This diff replaces some of the references to `ObjectIdAttributeNumber` with actual OIDs. # `kObjectId` is not available in upstream PG15 as OID is a regular column henceforth. This diff cleans the variable along with all of its usage. Test Plan: Jenkins: skip, rebase: pg15 Reviewers: smishra, jason Reviewed By: jason Subscribers: yql, ybase Differential Revision: https://phorge.dev.yugabyte.com/D28840
Summary: This diff fixes the following: # Reworked `CopyFrom` function as it was not merged properly due to conflicts. # Fixed `DoCopyTo` by switching out of temporary memory context - this was mistakenly skipped during the merge. The unused variable `oldcontext` was causing a build failure. # Cleanup: `PutMemoryContextsStatsTupleStore` and `pg_get_backend_memory_contexts ` were moved to `mcxtfuncs.c` but the original was not removed from `mcxt.c` during the merge. The unused functions were causing a build failure. Test Plan: Jenkins: rebase: pg15, test regex: org.yb.pgsql.TestPgRegress.* Added copy from test to temporary pg15 test suite. Note that it involves creating a partitioned table that needs D28432. Also, locally tested the diff with regression test `yb_feature_copy`. Reviewers: tvesely Reviewed By: tvesely Subscribers: dsrinivasan, mihnea, yql Differential Revision: https://phorge.dev.yugabyte.com/D28500
Summary: This diff fixes the following: # PG ( in [[ postgres/postgres@c2fe139 | commit ]]) replaced `heap_beginscan` with `table_beginscan` and `HeapTuple` with `TupleTableSlot` in `validateForeignKeyConstraint`. This was not merged properly with YB and is handled in this diff. # PG (in [[ postgres/postgres@4c9e1bd | commit ]]) resets memory context once per tuple in `validateForeignKeyConstraint`. This change was not merged properly during the YB and PG15 merge. This diff handles it. # PG (in [[ postgres/postgres@d6f96ed | commit ]]) added a new feature "Allow specifying column list for foreign key ON DELETE SET actions". The corresponding change to `YbATCreateSimilarForeignKey` is done in this diff. # PG (in [[ postgres/postgres@f56f8f8#diff-d373fb790605aded212c2e3a0fbe8c015d110077ff46c07d0a6d6647a2d7e928 | commit ]]) removed the function `createForeignKeyTriggers`. This function was called in `YbATCreateSimilarForeignKey`. This is handled in this diff. # There is a silent bug in YB master branch: in function `YbGetNext`, `desc->pkrel` is being passed to `YbAddTriggerFKReferenceIntent` as `fk_rel` argument. This change was done in [[ yugabyte@abc262c | commit ]]. This came to light on running `ALTER TABLE .. ADD FOREIGN KEY` on pg15 branch because PG introduced a equality check on `riinfo->fk_relid` and `RelationGetRelid(trig_rel)` in [[ postgres/postgres@815b20a | commit ]]. As the bug (from YB master) and the check (from PG upstream) were couples in pg15 branch, adding FK threw "wrong pg_constraint entry for trigger" error. This diff fixes the bug. Test Plan: Jenkins: rebase: pg15, test regex: org.yb.pgsql.TestPgRegress.* Reviewers: dmitry Reviewed By: dmitry Subscribers: dmitry, yql Differential Revision: https://phorge.dev.yugabyte.com/D28808
Test Plan: Jenkins: rebase: pg15, test regex: org.yb.pgsql.TestPgRegress.* Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D28946
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.
No description provided.