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

Allow primary keys to contain address columns sorted by their resolved values. #1214

Merged
merged 7 commits into from
Feb 25, 2025

Conversation

nicktobey
Copy link
Contributor

This is the Doltgres part of this change.

GMS PR: dolthub/go-mysql-server#2854
Dolt PR: dolthub/dolt#8870

The goal of the change is to allow for indexes to use an out-of-line variable-length type (like TEXT or BLOB) as a primary key while still storing just the address in the index (instead of being forced to store a prefix of the value).

As a result of this change, any tuple comparison operation may need to resolve a hash in the NodeStore. This poses two complications:

  1. The tuple logic exists at a much lower level than the node store and can't depend on it without creating a dependency cycle. We get around this with a new ValueStore interface that can store and retrieve variable-length bytestrings by their content hash. NodeStore is the only implementation of this interface, but decoupling the interface from the implementation allows us to not depend on NodeStore's internals when passing it to lower-level code.

  2. Tuple comparison operations can now end up doing disk IO, which means they need a context parameter.

Copy link
Contributor

github-actions bot commented Feb 19, 2025

Main PR
covering_index_scan_postgres 360.93/s ${\color{red}DNF}$
index_join_postgres 152.92/s ${\color{red}DNF}$
index_join_scan_postgres 184.43/s ${\color{red}DNF}$
index_scan_postgres 12.48/s ${\color{red}DNF}$
oltp_point_select 2825.16/s ${\color{red}DNF}$
oltp_read_only 1889.91/s ${\color{red}DNF}$
select_random_points 112.43/s ${\color{red}DNF}$
select_random_ranges 131.19/s ${\color{red}DNF}$
table_scan_postgres 11.22/s ${\color{red}DNF}$
types_table_scan_postgres 5.58/s ${\color{red}DNF}$

Copy link
Contributor

github-actions bot commented Feb 19, 2025

Main PR
Total 42090 42090
Successful 15654 15657
Failures 26436 26433
Partial Successes1 5212 5213
Main PR
Successful 37.1917% 37.1989%
Failures 62.8083% 62.8011%

${\color{lightgreen}Progressions (3)}$

domain

QUERY: select '{1,2,3}'::dia;

with

QUERY: SELECT count(*) FROM (
    WITH RECURSIVE t(n) AS (
        SELECT 1 UNION ALL SELECT n + 1 FROM t WHERE n < 500
    )
    SELECT * FROM t) AS t WHERE n < (
        SELECT count(*) FROM (
            WITH RECURSIVE t(n) AS (
                   SELECT 1 UNION ALL SELECT n + 1 FROM t WHERE n < 100
                )
            SELECT * FROM t WHERE n < 50000
         ) AS t WHERE n < 100);
QUERY: WITH RECURSIVE
   x(id) AS
     (VALUES (1) UNION ALL SELECT id+1 FROM x WHERE id < 5),
   y(id) AS
     (VALUES (1) UNION ALL SELECT id+1 FROM x WHERE id < 10)
 SELECT y.*, x.* FROM y LEFT JOIN x USING (id);

Footnotes

  1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct.

Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

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

LGTM

@nicktobey nicktobey force-pushed the nicktobey/textindex branch 2 times, most recently from 3cc9184 to 62fc2a3 Compare February 25, 2025 03:45
@nicktobey
Copy link
Contributor Author

#1224 shows that CI passes once this gets combined with the changes from #1219

@nicktobey nicktobey enabled auto-merge February 25, 2025 20:01
@nicktobey nicktobey disabled auto-merge February 25, 2025 20:01
@nicktobey nicktobey merged commit 06bef14 into main Feb 25, 2025
1 of 14 checks passed
@nicktobey nicktobey deleted the nicktobey/textindex branch February 25, 2025 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants