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

Fix handling for multiple unique index in compressed INSERT #7061

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

svenklemm
Copy link
Member

@svenklemm svenklemm commented Jun 25, 2024

Adjust compressed_insert_key_columns to correctly handle multiple
unique indexes. This patch changes the function to no longer combine
the columns from multiple indexes but instead only return intersecting
columns from all the unique indexes.
This patch also fixes a couple comments in that function.

Disable-check: commit-count

@svenklemm svenklemm self-assigned this Jun 25, 2024
@svenklemm svenklemm added the bug label Jun 25, 2024
@svenklemm svenklemm force-pushed the compressed_insert_key_columns branch from 99aa76b to 2c59e76 Compare June 25, 2024 09:15
}
index_close(indexDesc, AccessShareLock);

if (first_iteration)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (first_iteration)
if (!shared_attrs)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that wouldnt handle the case when we have non-intersecting indexes resetting shared_atts

@svenklemm svenklemm force-pushed the compressed_insert_key_columns branch from 2c59e76 to 5bf3a41 Compare June 25, 2024 09:43
*
* This is based on RelationGetIndexAttrBitmap from postgres with changes
* to also track unique expression indexes.
* In case of multiple unique indexes we have to return the shared columns.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to understand this better: we only use one index for decompression, so it means we have to decompress all batches that match by shared columns, and we can't add a filter on the other columns, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is columns we can safely use for filtering the batches in INSERT to satisfy the UNIQUE checks. The check is based on the unique indexes defined on the uncompressed chunk. These columns might be indexed on the compressed because they are segmentby or have metadata and we can filter on them even if we dont have corresponding compressed index.

@svenklemm svenklemm force-pushed the compressed_insert_key_columns branch 2 times, most recently from 1698911 to 76f8912 Compare June 25, 2024 12:53
@svenklemm svenklemm enabled auto-merge (rebase) June 25, 2024 12:54
@svenklemm svenklemm force-pushed the compressed_insert_key_columns branch 4 times, most recently from adac3fb to e575c4c Compare June 25, 2024 16:59
Adjust compressed_insert_key_columns to correctly handle multiple
unique indexes. This patch changes the function to no longer combine
the columns from multiple indexes but instead only return intersecting
columns from all the unique indexes.
This patch also fixes a couple comments in that function.
Copy link

codecov bot commented Jun 25, 2024

Codecov Report

Attention: Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.

Project coverage is 81.75%. Comparing base (59f50f2) to head (e575c4c).
Report is 227 commits behind head on main.

Files Patch % Lines
tsl/src/compression/compression.c 72.72% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7061      +/-   ##
==========================================
+ Coverage   80.06%   81.75%   +1.68%     
==========================================
  Files         190      200      +10     
  Lines       37181    37298     +117     
  Branches     9450     9725     +275     
==========================================
+ Hits        29770    30493     +723     
+ Misses       2997     2897     -100     
+ Partials     4414     3908     -506     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@svenklemm svenklemm merged commit fb14771 into timescale:main Jun 25, 2024
37 of 38 checks passed
pallavisontakke added a commit to pallavisontakke/timescaledb that referenced this pull request Jun 26, 2024
This release contains performance improvements and bug fixes since
the 2.15.2 release. Best practice is to upgrade at the next
available opportunity.

**Features**

**Bugfixes**
* timescale#7061 Fix handling of multiple unique indexes in compressed INSERT

**Thanks**
@pallavisontakke pallavisontakke mentioned this pull request Jun 26, 2024
pallavisontakke added a commit to pallavisontakke/timescaledb that referenced this pull request Jun 27, 2024
This release contains performance improvements and bug fixes since
the 2.15.2 release. Best practice is to upgrade at the next
available opportunity.

**Features**

**Bugfixes**
* timescale#7061 Fix handling of multiple unique indexes in compressed INSERT

**Thanks**
@pallavisontakke pallavisontakke mentioned this pull request Jun 27, 2024
pallavisontakke added a commit to pallavisontakke/timescaledb that referenced this pull request Jul 2, 2024
This release contains performance improvements and bug fixes since
the 2.15.2 release. Best practice is to upgrade at the next
available opportunity.

**Features**

**Bugfixes**
* timescale#7061 Fix handling of multiple unique indexes in compressed INSERT
* timescale#7080 Fix `corresponding equivalence member not found` error

**Thanks**
@pallavisontakke pallavisontakke mentioned this pull request Jul 2, 2024
pallavisontakke added a commit to pallavisontakke/timescaledb that referenced this pull request Jul 2, 2024
This release contains bug fixes since the 2.15.2 release.
Best practice is to upgrade at the next available opportunity.

**Migrating from self-hosted TimescaleDB v2.14.x and earlier**

After you run `ALTER EXTENSION`, you must run [this SQL script](https://github.com/timescale/timescaledb-extras/blob/master/utils/2.15.X-fix_hypertable_foreign_keys.sql). For more details, see the following pull request [timescale#6797](timescale#6797).

If you are migrating from TimescaleDB v2.15.0, v2.15.1 or v2.15.2, no changes are required.

**Bugfixes**
* timescale#7061: Fix handling of multiple unique indexes in compressed INSERT.
* timescale#7080: Fix `corresponding equivalence member not found` error.
@pallavisontakke pallavisontakke mentioned this pull request Jul 2, 2024
pallavisontakke added a commit to pallavisontakke/timescaledb that referenced this pull request Jul 2, 2024
This release contains bug fixes since the 2.15.2 release.
Best practice is to upgrade at the next available opportunity.

**Migrating from self-hosted TimescaleDB v2.14.x and earlier**

After you run `ALTER EXTENSION`, you must run [this SQL script](https://github.com/timescale/timescaledb-extras/blob/master/utils/2.15.X-fix_hypertable_foreign_keys.sql). For more details, see the following pull request [timescale#6797](timescale#6797).

If you are migrating from TimescaleDB v2.15.0, v2.15.1 or v2.15.2, no changes are required.

**Bugfixes**
* timescale#7061: Fix handling of multiple unique indexes in compressed INSERT.
* timescale#7080: Fix `corresponding equivalence member not found` error.
pallavisontakke added a commit that referenced this pull request Jul 2, 2024
This release contains bug fixes since the 2.15.2 release.
Best practice is to upgrade at the next available opportunity.

**Migrating from self-hosted TimescaleDB v2.14.x and earlier**

After you run `ALTER EXTENSION`, you must run [this SQL
script](https://github.com/timescale/timescaledb-extras/blob/master/utils/2.15.X-fix_hypertable_foreign_keys.sql).
For more details, see the following pull request
[#6797](#6797).

If you are migrating from TimescaleDB v2.15.0, v2.15.1 or v2.15.2, no
changes are required.

**Bugfixes**
* #7061: Fix the handling of multiple unique indexes in a compressed
INSERT.
* #7080: Fix the `corresponding equivalence member not found` error.
* #7088: Fix the leaks in the DML functions.
* #7035: Fix the error when acquiring a tuple lock on the OSM chunks on
the replica.

**Thanks**
* @Kazmirchuk for reporting the issue about leaks with the functions in
DML.
pallavisontakke added a commit to pallavisontakke/timescaledb that referenced this pull request Jul 2, 2024
This release contains performance improvements and bug fixes since
the 2.15.2 release. Best practice is to upgrade at the next
available opportunity.

**Features**

**Bugfixes**
* timescale#7061 Fix handling of multiple unique indexes in compressed INSERT
* timescale#7080 Fix `corresponding equivalence member not found` error
* timescale#7088 Fix leaks with functions in DML

**Thanks**
* @Kazmirchuk for reporting this
@pallavisontakke pallavisontakke mentioned this pull request Jul 2, 2024
svenklemm pushed a commit to pallavisontakke/timescaledb that referenced this pull request Jul 2, 2024
This release contains performance improvements and bug fixes since
the 2.15.2 release. Best practice is to upgrade at the next
available opportunity.

**Features**

**Bugfixes**
* timescale#7061 Fix handling of multiple unique indexes in compressed INSERT
* timescale#7080 Fix `corresponding equivalence member not found` error
* timescale#7088 Fix leaks with functions in DML

**Thanks**
* @Kazmirchuk for reporting this
svenklemm added a commit that referenced this pull request Jul 2, 2024
This release contains bug fixes since the 2.15.2 release.
Best practice is to upgrade at the next available opportunity.

**Migrating from self-hosted TimescaleDB v2.14.x and earlier**

After you run `ALTER EXTENSION`, you must run [this SQL
script](https://github.com/timescale/timescaledb-extras/blob/master/utils/2.15.X-fix_hypertable_foreign_keys.sql).
For more details, see the following pull request
[#6797](#6797).

If you are migrating from TimescaleDB v2.15.0, v2.15.1 or v2.15.2, no
changes are required.

**Bugfixes**
* #7061: Fix the handling of multiple unique indexes in a compressed
INSERT.
* #7080: Fix the `corresponding equivalence member not found` error.
* #7088: Fix the leaks in the DML functions.
* #7035: Fix the error when acquiring a tuple lock on the OSM chunks on
the replica.
* #7091: Fix ORDER BY/GROUP BY expression not found in targetlist on
PG16

**Thanks**
* @Kazmirchuk for reporting the issue about leaks with the functions in
DML.

---------

Signed-off-by: Sven Klemm <31455525+svenklemm@users.noreply.github.com>
Co-authored-by: Sven Klemm <31455525+svenklemm@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants