Row-level partitioning support in YSQL #4761
Closed
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
deeps1991
changed the title
m-iancu
reviewed
Jul 7, 2020
| 400 | 0 | 0000 | | | | | | ||
| 450 | 0 | 0002 | 450 | 0002 | 0 | 450 | 450 | ||
| 500 | 0 | 0000 | | | | | | ||
| 550 | 0 | 0002 | | | | | |
| -- resulting from the scan of pg_depend preventing the delete of the trigger. The trigger | ||
| -- depends on the partition table itself and on the trigger created on the partitioned | ||
| -- table, and either of the two error messages are possible. | ||
| /* |
There was a problem hiding this comment.
For this cases, instead of disabling them, we temporarily change verbosity level for these commands to omit the detail lines.
For instance:
\set verbosity terse
DROP ..
\set verbosity default
Alternatively, you can use the native SQL way of doing SET client_min_messages TO 'WARNING' and then RESET client_min_messages.
The latter is probably cleaner if it works because it's just native SQL (rather than a ysqlsh command) but (at least) one of them typically works to stabilize test output in these cases.
There was a problem hiding this comment.
The problem here was that was differing error messages... And there doesn't seem to be a way to silence Error messages as far as I could google :( Is there anything else I can do?
m-iancu
reviewed
Jul 7, 2020
| @@ -796,9 +796,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, | |||
| InvalidOid, | |||
| typaddress); | |||
|
|
|||
| /* Store inheritance information for new rel. */ | |||
There was a problem hiding this comment.
I have explained this in detail in the commit message. I think this is a mistaken merge conflict artifact, as I couldn't see this line in Postgres 11.2 code.
…EST_ prefix
Summary:
We pretty much internalize that test flags should start with a TEST_ prefix. We should
probably add some automation for that. In this diff I just made sure current flags respect that.
Made a couple of minor manual edits before (bring some flag names to the same line as where they
are defined), as well as after (to fix a flag called `running_test`). Else this oneliner fixed the
rest:
`git grep 'DEFINE_test_flag' | grep -v TEST_ | grep -vE '.*, name, .*' | sed 's/.*DEFINE_test_flag([^,]*, \([^,]*\),.*/\1/g' | while read flag; do git grep -l $flag | xargs -I{} sed -i "s/$flag/TEST_$flag/g" {}; done`
Test Plan: jenkins
Reviewers: sergei, timur, mikhail
Reviewed By: mikhail
Subscribers: ybase
Differential Revision: https://phabricator.dev.yugabyte.com/D8562
…g a leader stepdown Summary: When a leader stepdown happens with no new leader candidate specified in the stepdown request, the peer simply steps down leaving the group with no leader until regular hearbeat timeouts are triggered. This change makes it so that the leader attempts to transition to the most up to date peer if possible. The main driver for this change is that when load balancer removes tablets off a blacklisted node, it triggers a leader stepdown with no leader candidate specified. During leader moves too, the load balancer cannot be assured that the new leader it has selected is actually a viable candidate to win the election (i.e. is the most advanced peer). The new default is to always find a new leader during stepdown unless a specific flag is specified to turn off this behavior. The flag can be set for a yb-admin triggered stepdown if needed. (Should we also set a similar flag for the load balancer triggered step downs?). Test Plan: 1. The existing leader stepdown unit test was enhanced to test this change. 2. I manually tested that a yb-sample-apps workload running during a leader stepdown event (triggered via yb-admin) saw timeouts before this change but not afterwards. Here are the testing parameters used: ``` $ bin/yb-ctl destroy && bin/yb-ctl start --rf 3 --master_flags 'load_balancer_max_concurrent_moves=0,"vmodule=cluster_balance*=1,catalog_manager*=1,catalog_entity_info*=1"' --tserver_flags 'leader_failure_max_missed_heartbeat_periods=200' $ java -jar ~/code/yb-sample-apps/target/yb-sample-apps.jar --workload CassandraKeyValue --nodes 127.0.0.1:9042 --num_threads_read 1 --num_threads_write 1 --cql_connect_timeout_ms 1000 --cql_read_timeout_ms 1000 $ yb-admin --init_master_addrs=127.0.0.1:7100 leader_stepdown 1932e37001bd4fe8934d460cc0a992da ``` Reviewers: hector, rahuldesirazu, bogdan, sergei Reviewed By: bogdan, sergei Subscribers: kannan, sergei, ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8440
…test/de… (yugabyte#4714) * add `set_preferred_zones` in 3dc deployment
* add replication lag script in 2DC
…ED entries yugabyte#4351 (yugabyte#4475) * yugabyte#4351 Add flag for list_snapshots * yugabyte#4351 Add flag for list_snapshots * yugabyte#4351 Add flag for list_snapshots * yugabyte#4351 Add flag for list_snapshots * yugabyte#4351 Add flag for list_snapshots * yugabyte#4351 Add flag for list_snapshots * yugabyte#4351 Add flag for list_snapshots
Summary: Remove read causing use-after-poison in 2 tests. Also fixed undefined behavior when bit shifting with invalid value: `SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../src/yb/yql/pgwrapper/pg_libpq-test.cc:507:33 in·` Test Plan: `ybd asan --cxx-test pgwrapper_pg_libpq-test --gtest_filter PgLibPqTest.MultiBankAccountSerializable -n 50 --tp 1` Reviewers: amitanand, hector, raju Reviewed By: raju Subscribers: ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8642
…s to openssl, curl from thirdparty Summary: The patch to squeasel (yugabyte/yugabyte-db-thirdparty@764fea2) changed the return type of get_bound_addresses to be sockaddr_storage that is big enough to hold both IPv4 and IPv6 addresses. This diff uses this API. The patch also enabled IPv6 address handling in squeasel. Test Plan: * Verified master web ui looks good on linux dev server * Built a release package and deployed on portal, Ran sample apps SQL and CQL workloads. * Jenkins Reviewers: bogdan, sergei, mikhail Reviewed By: mikhail Subscribers: ybase, bogdan Differential Revision: https://phabricator.dev.yugabyte.com/D8616
Summary: Remove StopWatch from compaction iterator to avoid two time queries per compaction filter call. Subset of changes in https://phabricator.dev.yugabyte.com/D8604, authored by @mbautin Benchmark Results: ./yb_build.sh release --cxx-test docdb_randomized_docdb-test --gtest_filter bool/RandomizedDocDBTest.Snapshots/0 -n 10 -- -p 1 -k Before: 14656.4 ms, After: 14423.5 ms, Gain: 1.02x ./yb_build.sh release --cxx-test docdb_randomized_docdb-test --gtest_filter bool/RandomizedDocDBTest.Snapshots/1 -n 10 -- -p 1 -k Before: 5798.1 ms, After: 5715.8 ms, Gain: 1.01x ------------------------ ./yb_build.sh release --cxx-test docdb_randomized_docdb-test --gtest_filter bool/RandomizedDocDBTest.SnapshotsWithHistoryCleanup/0 -n 10 -- -p 1 -k Before: 9929.9 ms, After: 9841.2 ms, Gain: 1.01x ./yb_build.sh release --cxx-test docdb_randomized_docdb-test --gtest_filter bool/RandomizedDocDBTest.SnapshotsWithHistoryCleanup/1 -n 10 -- -p 1 -k Before: 5314.5 ms, After: 5127.1 ms, Gain: 1.04x ------------------------ ./yb_build.sh release --cxx-test pg_mini-test --gtest_filter PgMiniTest.BigRead -n 10 -- -p 1 -k Before: 93300.3 ms, After: 92834.1 ms, Gain: 1.0x ./yb_build.sh release --cxx-test pg_mini-test --gtest_filter PgMiniTest.BigReadWithCompaction -n 10 -- -p 1 -k Before: 97169.5 ms, After: 96333.1 ms, Gain: 1.01x ./yb_build.sh release --cxx-test pg_mini-test --gtest_filter PgMiniTest.SmallRead -n 10 -- -p 1 -k Before: 7108 ms, After: 6782.3 ms, Gain: 1.05x ------------------------ Setup: Ubuntu 18.04, GCP n1-standard-4 Test Plan: jenkins Reviewers: bogdan, mikhail Reviewed By: mikhail Subscribers: ybase, kannan, sergei Differential Revision: https://phabricator.dev.yugabyte.com/D8649
Summary: Remove the bitshuffle dependency that we don't seem to be using. Remove bz2 from the list of libraries RocksDB is being linked with. Don't look for flex and bison in Homebrew directories. Those tools are now included in thirdparty. Test Plan: Jenkins: compile only Reviewers: sergei, bogdan, neil, sanketh Reviewed By: sanketh Subscribers: ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8656
…ailable due to pending task.
Summary:
When universe has a pending task and masterAddresses are empty, the fetchUniverseTablesList
call returns a string instead of array. From TablesController.java
```
if (masterAddresses.isEmpty()) {
String errMsg = "Expected error. Masters are not currently queryable.";
LOG.warn(errMsg);
return ok(errMsg);
}
```
Because we expect an array, calling `.forEach` will throw an error.
Check that data is an array and use empty array as default if it is not.
Test Plan: Navigate to universe Backups tab when there is a pending task.
Reviewers: ram, sshevchenko
Reviewed By: sshevchenko
Subscribers: ui, rao
Differential Revision: https://phabricator.dev.yugabyte.com/D8663
…m_tracker_hard_limit and rpc_throttle_threshold_bytes Summary: Since we've enabled these in some of our cluster testing, we should just enable these by default. Test Plan: jenkins Reviewers: timur Reviewed By: timur Subscribers: ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8646
Summary: Following log messages looks confusing and periodically treated as errors by users: ``` I0611 06:23:58.795145 12268 reactor.cc:450] Master_R003: Timing out connection Connection (0x00000000027afc30) server 10.132.0.2:38383 => 10.128.0.11:7100 - it has been idle for 65.0003s (delta: 65.0003, current time: 34168.1, last activity time: 34103.1) I0611 06:23:58.795548 12267 reactor.cc:450] Master_R002: Timing out connection Connection (0x00000000027af210) server 10.132.0.2:38643 => 10.128.0.11:7100 - it has been idle for 65.0993s (delta: 65.0993, current time: 34168.1, last activity time: 34103) I0611 06:23:58.895499 12270 reactor.cc:450] Master_R005: Timing out connection Connection (0x00000000022cb7b0) server 10.132.0.2:59863 => 10.128.0.11:7100 - it has been idle for 65.0003s (delta: 65.0003, current time: 34168.2, last activity time: 34103.2) ``` Updated it to be like: ``` I0611 06:23:58.795145 12268 reactor.cc:450] Master_R003: DEBUG: Closing idle connection: Connection (0x00000000027afc30) server 10.132.0.2:38383 => 10.128.0.11:7100 - it has been idle for 65.0003s I0611 06:23:58.795548 12267 reactor.cc:450] Master_R002: DEBUG: Closing idle connection: Connection (0x00000000027af210) server 10.132.0.2:38643 => 10.128.0.11:7100 - it has been idle for 65.0993s I0611 06:23:58.895499 12270 reactor.cc:450] Master_R005: DEBUG: Closing idle connection: Connection (0x00000000022cb7b0) server 10.132.0.2:59863 => 10.128.0.11:7100 - it has been idle for 65.0003s ``` Test Plan: Jenkins Reviewers: bogdan, kannan Reviewed By: kannan Subscribers: ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8657
Summary: In case local file `thirdparty/.yb_thirdparty_do_not_use` exists, `bin/remote_build.py` script tries to sync `thirdparty/.git` onto remote server since thirdparty has been added as a submodule. This causes the remote build to fail. Added exclude rule for `rsync` call to avoid syncing `.git`. Test Plan: `touch thirdparty/.yb_thirdparty_do_not_use` `./bin/remote_build.py --remote-path /nfusr/dev-server/timur/code/yugabyte2 --host dev-server-timur -- --remote --dltp` Jenkins: skip Reviewers: alex, sergei, mikhail Reviewed By: mikhail Subscribers: ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8665
…litting Summary: Implementation takes middle restart point from the largest SST file index and uses next DocKey from SST file key as an approximate middle DocKey for a tablet. Other changes: - `BlockBasedTable::GetIndexReader` code from `BlockBasedTable::NewIndexIterator` to reuse it in `BlockBasedTable::GetMiddleKey`. - Added `TableCache::GetTableReader` - Added `QLTabletTest.GetMiddleKey` - Added `BlockTest.GetMiddleKey` Test Plan: `ybd --gtest_filter QLTabletTest.GetMiddleKey -n 100 -- -p 1` Reviewers: sergei, mikhail, bogdan Reviewed By: bogdan Subscribers: raju, ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8636
Summary: Backoff waiter is used to wait between status calls in intent aware iterator. So we should NOT use its timing as request timeout. Because it could result in unnecessary request retry. This diff fixes the issue and introduce separate timeout for request. Test Plan: ybd debug --gtest_filter QLTransactionTest.ReadRestartWithPendingIntents -n 100 -- -p 12 Reviewers: bogdan, timur Reviewed By: timur Subscribers: ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8652
…d during transaction aware snapshot creation Summary: When the table is not running async RPC task would not be able to reset the tablet server proxy. But such a case is handled incorrectly and operation just retries after the timeout. This diff fixes the issue. Also fixed issue when a new master leader could miss changes performed by the previous leaders. Test Plan: ybd --gtest_filter BackupTxnTest.DeleteTableWithMastersRestart ybd --gtest_filter BackupTxnTest.CompleteAndBounceMaster Reviewers: mikhail, oleg, bogdan, amitanand, hector Reviewed By: amitanand Subscribers: kannan, ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8645
…ing for index covering.
Summary:
**Issue: **
The existing column name mangling method for the following INDEX is not working correctly.
CREATE TABLE tab (i INT PRIMARY KEY, v INT, vv INT, j jsonb)
CREATE INDEX idx ON a_table(v, j->>'a');
- Column "v" is mangled as "$C_v"
- Column "vv" is mangled as "$C_vv"
- Column "j->>'a'" is mangled as "$C_j->>'a'"
As a result, IsExprCovered("$C_vv") would return true because it interprets "$C_vv" as an expression of "$C_v" column where the extra "v" is an operator (similar to "$C_v+")
**Solution**
The right thing to do is to fix the name-mangling method, but that requires metadata to be updated, and we should only do that for major releases. Therefore, I've used the following quick fix for column name resolution.
Instead of using sub-string, the method in this fix is to collect names of columns and JSONB expression into a list and then checking that list against the INDEX definition.
- Traverse the expressions in SELECT to form a list of names of columns and JSONB expressions. For example { $C_vv, $C_j->>'a' }
- Check if the INDEX definitions have those column names. A SELECT statement is covered by an INDEX if {SELECT's names of COLs and JSONBs} is a subset of { INDEX's COLs and JSONBs }.
- I keep the code where the expression name / string is used to check for covering but deactivate it with "if (false)". Later on, we can revisit it.
The only limitation of this quick-fix is that will support only index by JSONB expression while the existing method support INDEX by any expressions.
**Example on the difference between existing code and this quick fix.**
```
SELECT j->>'a' + 77, v, vv FROM tab WHERE jsonb->>'a' = '77';
```
- The existing method would check if the string "j->>'a'" is a sub-string of the string "j->>'a' + 77". This sub-string method would work for all kinds of expressions and not just JSONB expression.
- The method in the quick fix would walk SELECT statement to construct a list of ONLY column names and JSONB expression such as ```{ j->>'a', v, vv }``` and then compare the INDEX column names against that list. Because the quick fix method collects only JSONB expressions besides column name, it only works for JSONB expression. To support other kinds of expressions, it'd need to be updated to add those expressions to the constructed list.
Test Plan: Run TestJsonIndex.java and TesIndex.java
Reviewers: mihnea
Reviewed By: mihnea
Subscribers: yql
Differential Revision: https://phabricator.dev.yugabyte.com/D8763
… of warehouses. (yugabyte#4915) Reviewers: Karthik, Neha
Summary:
This makes it easier to parse. The scenario where this is required is - To allow the k8s operator to automatically perform scale down operations, it needs to be able to look at the blacklist in the cluster_config. The current output is in protobuf DebugString() format and changing it to json would make it easier to parse.
I assume no one relies on the current format as it is right now, so it would be ok to do a breaking change instead of adding new flags.
Test Plan:
Run yb-admin, verify output
```
23:20 ~/code2/yugabyte-db [master] $ build/latest/bin/yb-admin --master_addresses=127.0.0.1:7100,127.0.0.2:7100,127.0.0.3:7100 get_universe_config 2> /dev/null
{"version":1,"serverBlacklist":{"hosts":[{"host":"127.0.0.1","port":7100}],"initialReplicaLoad":0},"clusterUuid":"d3cebbdb-ec75-411f-a9c2-38c6ab527c90"}
```
Reviewers: bogdan, hector, raju
Reviewed By: hector, raju
Subscribers: ybase
Differential Revision: https://phabricator.dev.yugabyte.com/D8601
Summary: It appears that `BACKFILL INDEX` statement syntax rule was removed by commit 793c1a0. Add it back in. Close: yugabyte#4933 Test Plan: ```sh ./yb_build.sh \ --cxx-test pgwrapper_pg_libpq-test \ --gtest_filter 'PgLibPqTest.Backfill*' ``` Reviewers: timur, ena Reviewed By: ena Subscribers: yql Differential Revision: https://phabricator.dev.yugabyte.com/D8786
Summary: Add test cases where non-covering columns are listed in the filter (WHERE clause). These additional tests are the following commit. yugabyte@72af174 Test Plan: TestJsonIndex Reviewers: mihnea Reviewed By: mihnea Subscribers: kannan, yql Differential Revision: https://phabricator.dev.yugabyte.com/D8789
Summary: Our current code did not create client certs for user provided certs, causing universe creation to fail due to lack of the client cert files. Test Plan: Created a universe with a user provided cert and verified that it worked as expected. Also added unit tests. Reviewers: sanketh, ram, daniel Reviewed By: ram, daniel Subscribers: jenkins-bot, daniel, yugaware Differential Revision: https://phabricator.dev.yugabyte.com/D8691
Summary: Add `CreateTableRequestPB.skip_index_backfill` so that certain statements can create indexes without needless overhead from online schema migration (e.g. `CREATE TABLE` with unique column constraint). Also, increase compatibility with currently unsupported statements by falling back to skipping index backfill: - drop index - index created in postgres nested DDL - unique index Depends on D8771 Close: yugabyte#4918 Test Plan: ```sh ./bin/yb-ctl create \ --master_flags "ysql_disable_index_backfill=false" \ --tserver_flags "ysql_disable_index_backfill=false" ./bin/ysqlsh --command "CREATE TABLE t (i int, j int, UNIQUE (i))" ./bin/ysqlsh --command "CREATE UNIQUE INDEX ON t (j)" ./bin/ysqlsh --command "DROP INDEX t_j_idx" ``` Reviewers: neha, mihnea Reviewed By: mihnea Subscribers: yql, bogdan Differential Revision: https://phabricator.dev.yugabyte.com/D8778
…download a snapshot Summary: Currently, if we have any failure during the download process (RocksDB files, WAL files, or snapshots), the remote bootstrap session gets aborted. We noticed that if a snapshot process fails, it can leave some data behind, and during the remote bootstrap, the download stage could fail, making the whole session get aborted, and entering into a never ending cycle. This diff fixes that issue by ignoring errors that happen while downloading snapshot files. If we fail to download a snapshot file, the receiver deletes the snapshot directory and stops attempting to download the rest of the files for this failed snapshot. Remote bootstraps of other snapshots don't get affected. Test Plan: New unit test Reviewers: oleg, bogdan, timur Reviewed By: timur Subscribers: sergei, ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8757
…#4822) * [YSQL] Use default db as yugabyte irrespective of user name
… random free port Summary: In PgMiniTest the pgsql web server needs to bind to a random free port as opposed to always using port 13000. We currently do that using a gflag rather than using a field in the PgProcessConf struct. In pg_mini-test, we bring up only one postgres server but multiple tservers so just setting the pgsql webserver flag to an unallocated port is sufficient. Also remove the unused mini_cluster_base_port flag. Test Plan: - Jenkins - Manually start a cluster using yb-ctl and make sure it binds to port 13000. Then run the PgMiniTest.Simple test and verify in the logs that it picks a random free port for the pgsql webserver. Reviewers: bogdan, neha, sanketh Reviewed By: sanketh Subscribers: ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8794
Summary: The original fix for issue yugabyte#4881 introduced another bug (in the drop-column case for alter table). So we revert the original fix and a follow-up diff with additional tests. The referenced issue (yugabyte#4881) will be addressed soon in a follow-up diff with a corrected fix. Revert "[YSQL] yugabyte#4881 Additional test cases" This reverts commit 176bbc5. Revert "[YCQL] yugabyte#4881 Corrected name resolution for columns when checking for index covering." This reverts commit 72af174. Test Plan: Existing Jenkins tests. Reviewers: mihnea Reviewed By: mihnea Subscribers: yql Differential Revision: https://phabricator.dev.yugabyte.com/D8800
Summary:
Diff D8429 changed the cql_export metric to tserver_export metric. This made the platform
not correctly scrape the metric. This diff now looks for both export names to work with newer as
well as older YB universes.
Test Plan:
Created a universe with YB version before change and checked the metrics are visible.
{F13761}
Upgraded the universe to YB version after change and checked the metrics are still visible.
{F13762}
Reviewers: bogdan
Reviewed By: bogdan
Subscribers: jenkins-bot, yugaware
Differential Revision: https://phabricator.dev.yugabyte.com/D8802
…ing for index covering.
Summary:
**Issue: **
The existing column name mangling method for the following INDEX is not working correctly.
CREATE TABLE tab (i INT PRIMARY KEY, v INT, vv INT, j jsonb)
CREATE INDEX idx ON a_table(v, j->>'a');
- Column "v" is mangled as "$C_v"
- Column "vv" is mangled as "$C_vv"
- Column "j->>'a'" is mangled as "$C_j->>'a'"
As a result, IsExprCovered("$C_vv") would return true because it interprets "$C_vv" as an expression of "$C_v" column where the extra "v" is an operator (similar to "$C_v+")
**Solution**
The right thing to do is to fix the name-mangling method, but that requires metadata to be updated, and we should only do that for major releases. Therefore, I've used the following quick fix for column name resolution.
Instead of using sub-string, the method in this fix is to collect names of columns and JSONB expression into a list and then checking that list against the INDEX definition.
- Traverse the expressions in SELECT to form a list of names of columns and JSONB expressions. For example { $C_vv, $C_j->>'a' }
- Check if the INDEX definitions have those column names. A SELECT statement is covered by an INDEX if {SELECT's names of COLs and JSONBs} is a subset of { INDEX's COLs and JSONBs }.
- I keep the code where the expression name / string is used to check for covering but deactivate it with "if (false)". Later on, we can revisit it.
The only limitation of this quick-fix is that will support only index by JSONB expression while the existing method support INDEX by any expressions.
**Side Effect**
ALTER TABLE processing uses IsColumnCovered( check_by_column_id ) to determine if an index is dependent on a column. This is not enough after this bug is fixed. A new function CheckColumnDependency ( column_id ) is added for this purpose. For INDEX ( jsonb_col->>'field' ), although column "jsonb_col" is NOT covered by the INDEX, CheckColumnDependency( jsonb_col ) will return TRUE, and IsColumnCovered( jsonb_col ) will return FALSE.
**Example on the difference between existing code and this quick fix.**
```
SELECT j->>'a' + 77, v, vv FROM tab WHERE jsonb->>'a' = '77';
```
- The existing method would check if the string "j->>'a'" is a sub-string of the string "j->>'a' + 77". This sub-string method would work for all kinds of expressions and not just JSONB expression.
- The method in the quick fix would walk SELECT statement to construct a list of ONLY column names and JSONB expression such as ```{ j->>'a', v, vv }``` and then compare the INDEX column names against that list. Because the quick fix method collects only JSONB expressions besides column name, it only works for JSONB expression. To support other kinds of expressions, it'd need to be updated to add those expressions to the constructed list.
Test Plan: TesIndex TestJsonIndex
Reviewers: mihnea
Reviewed By: mihnea
Subscribers: yql
Differential Revision: https://phabricator.dev.yugabyte.com/D8799
…w_path.txt files Summary: Helpful when calling cmake directly, e.g., from an IDE such as CLion. Test Plan: ./yb_build.sh Jenkins: compile only Would also like test from a CLion user. Reviewers: alex, sergei, mikhail Reviewed By: mikhail Subscribers: devops Differential Revision: https://phabricator.dev.yugabyte.com/D8803
…s for cdc replication Summary: Currently when deciding if we can setup replication for a specific table, we check that the producer's table schema is equal to the consumer's table schema. This equals comparison, also check that the properties of both tables are the same. But for some properties, we don't need them to be the same. For example, if one table has the ttl property unset, while the other one has it set to 0, then it's perfectly fine to setup the replication between them. The same applies to other properties that don't affect replication like wal_retention_secs_ and is_backfilling. Test Plan: Modified the unit tests so that the consumer's table property has the TTL field set where as the producer's table property doesn't. Ran the modified tests without the new Equivalent method and 34 tests failed with `Source and target schemas don't match`. With the new modifications all the unit tests passed Reviewers: rahuldesirazu, neha, nicolas Reviewed By: nicolas Subscribers: ybase, bogdan Differential Revision: https://phabricator.dev.yugabyte.com/D8661
This patch removes all the modifications made to the grammar that disables any partitions related syntax.
…le creation. This causes a bug in the following manner: i) Child table is created on the heap, but no partition information about it has been updated in the catalog yet. ii) StoreCatalogInheritance is called, creating an entry for it in pg_inherits, but no information about its partition bounds is stored in pg_class iii) When StorePartitionBound is called, postgres tries to check whether the parent table has any child with an overlapping range iv) POstgres sees the newly created child table in pg_inherits v) However, when it tries to look up its bounds information in the catalog, there is understandably no bounds information for the new table (as we haven't updated it yet). An assertion fires, stating that there is a child table with no bounds information. To fix this issue, removed the extraneous StoreCatalogInheritance call. Also noticed that postgres 11.2 source code actually does not have this extraneous call.
Today YBPreloadRelCache during initialization, has two phases: 1) Scan pg_class table 2) Scan pg_attribute table This means the first scan populates some relations into the cache without their corresponding attribute information. This is usually okay, but this breaks once partitions are enabled. This is because, in the first phase, there was a special handling for partitioned tables that tried to update the partition key and bounds information. This requires the attribute info for pg_partitioned_table and pg_type catalog tables. However, since we were still in the first phase of YBPreloadRelCache, we will hit assertion here as these catalog tables will be present in the cache without any attribute information. To fix this, this patch does not update the partition information in the first phase. Since we need the attribute info populated for multiple catalog tables, and we cannot be sure whether these info were all populated when we try to process the partitioned table in the second phase, this patch creates a third phase. In the third phase, we scan the pg_partitioned_table, and update the cache for any partitioned tables with partition information.
…ables, any join queries where failing. This is because postgres was picking a strategy MergeAppend to combine values from multiple child tables. MergeAppend expects the input streams to be merged to be sorted. However, in YB, by default, any single column indices are hash based and unordered. Still, MergeAppend was treating the values returned by IndexScans as ordered. This is because try to pick hash index if there is an equality comparison in the equivalence class. However, the corresponding pathkey is created with strategy BTLessStrategyNumber or BTGreaterStrategyNumber always depending on the value passed for reverse_sort. Thus, when hash indices were picked, the merge append would assume that the output returned by this pathkey was already sorted appropriately. Fixed this by ensuring that when we pick a hash index, the corresponding pathkey should always have strategy BTEqualStrategyNumber. This way, the output from this pathkey is not considered to be ordered, and partition queries start returning correct results.
…gregate.sql in two new files yb_partition_join.sql and yb_partition_aggregate.sql. These files contain the same queries as the original postgres tests, but queries of the form "EXPLAIN.." and "ALTER TABLE" have been omitted as they are expected to fail. All other queries testing partition aggregates and partition joins work as expected and are included in the new files.
such that the partition constraint is no longer satisfied, PostgreSQL converts the operation into 2 operations - a DELETE from the current partition, and an INSERT into the new partition, however this is not a transaction. The check for row movement, and conversion into a DELETE+INSERT was disabled for Yugabyte tables, so the update was returning success without actually doing anything. In this patch, returning failure if partition constraints are being violated. In a later patch, this row movement will be performed as a transaction between the two partition tables. Additionally there were a few bugs in the UPDATE path owing to the fact that there were some occurrences of FirstLowInvalidHeapAttributeNumber were not replaced by the YB equivalent in the partitions code path. Fixed these bugs and added more tests in the update code path in this commit.
deeps1991
force-pushed
the
update-final-fix
branch
from
f72e76d
to
71d4033
Compare
|
|
|
I seem to have messed something up while rebasing and pushing, will submit a new pull request. Sorry about the confusion! |
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.