Skip to content

Root Object Rework#1265

Merged
Hydrocharged merged 1 commit intomainfrom
daylon/root-merging
Mar 28, 2025
Merged

Root Object Rework#1265
Hydrocharged merged 1 commit intomainfrom
daylon/root-merging

Conversation

@Hydrocharged
Copy link
Copy Markdown
Collaborator

@Hydrocharged Hydrocharged commented Mar 10, 2025

This reworks root objects so that they actually work, as they previously did not (outside of the session cache).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 13, 2025

Main PR
covering_index_scan_postgres 374.63/s 348.94/s -6.9%
index_join_postgres 153.51/s 151.94/s -1.1%
index_join_scan_postgres 185.61/s 183.61/s -1.1%
index_scan_postgres 12.49/s 12.47/s -0.2%
oltp_point_select 2799.85/s 2549.60/s -9.0%
oltp_read_only 1866.35/s 1767.89/s -5.3%
select_random_points 109.19/s 108.43/s -0.7%
select_random_ranges 130.44/s 130.30/s -0.2%
table_scan_postgres 10.25/s 10.38/s +1.2%
types_table_scan_postgres 5.32/s 5.32/s 0.0%

Copy link
Copy Markdown
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.

Just gave this is a pretty quick look, mostly was paying attention to the interface defined on the Dolt side. Overall design and structure seems straighforward, looking forward to seeing finalized storage details.

@Hydrocharged Hydrocharged force-pushed the daylon/root-merging branch 2 times, most recently from fad1032 to 0420d36 Compare March 24, 2025 13:14
@Hydrocharged Hydrocharged changed the title [WIP] Root Object Rework Root Object Rework Mar 25, 2025
@Hydrocharged Hydrocharged marked this pull request as ready for review March 25, 2025 13:13
@Hydrocharged Hydrocharged force-pushed the daylon/root-merging branch 3 times, most recently from 9bd1e2a to c054036 Compare March 25, 2025 14:06
@Hydrocharged
Copy link
Copy Markdown
Collaborator Author

Performance has gone down a little bit compared to running this locally, and I have some ideas on how it’s happening.

@Hydrocharged
Copy link
Copy Markdown
Collaborator Author

Regression tests are segfaulting from go-icu-regex. We did make that seemingly safe/simple change recently (and bumps have been broken since then), so maybe it's not allowed? Going to dig into that.

Copy link
Copy Markdown
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.

I haven't looked at every part of this yet, but left some high-level feedback for organization and interfaces.

The other feedback is that you should be sparing with calls to rootobject.Resolve, since that does several map iterations that are unnecessary in the common case.

I'll take a look again when you have a clean PR with passing tests, hopefully tomorrow am

Copy link
Copy Markdown
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.

Will take a final pass at this tomorrow morning

@github-actions
Copy link
Copy Markdown
Contributor

Main PR
Total 42090 42090
Successful 15759 15568
Failures 26331 26522
Partial Successes1 5201 5208
Main PR
Successful 37.4412% 36.9874%
Failures 62.5588% 63.0126%

${\color{red}Regressions (203)}$

alter_table

QUERY:          delete from atacc1;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          delete from atacc1;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          delete from parent;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          delete from child;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          delete from atacc1;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          select * from myview;
RECEIVED ERROR: expected row count 0 but received 3
QUERY:          alter table atacc1 drop b;
RECEIVED ERROR: duplicate primary key given: [] (errno 1062) (sqlstate HY000)
QUERY:          select * from atacc1;
RECEIVED ERROR: expected column count 0 but received 1
QUERY:          TRUNCATE old_system_table;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          CREATE TYPE mytype AS (a int);
RECEIVED ERROR: type "mytype" already exists (errno 1105) (sqlstate HY000)
QUERY:          DELETE FROM part_2;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          DELETE FROM part1;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          DELETE FROM hpart_2;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)

brin

QUERY:          DELETE FROM brintest_3;
RECEIVED ERROR: table not found: check_seq (errno 1146) (sqlstate HY000)

cluster

QUERY:          DELETE FROM clstr_1;
RECEIVED ERROR: table not found: check_seq (errno 1146) (sqlstate HY000)
QUERY:          DELETE FROM clstr_2;
RECEIVED ERROR: table not found: check_seq (errno 1146) (sqlstate HY000)
QUERY:          DELETE FROM clstr_3;
RECEIVED ERROR: table not found: check_seq (errno 1146) (sqlstate HY000)
QUERY:          INSERT INTO clstr_1 VALUES (2);
RECEIVED ERROR: duplicate primary key given: [2] (errno 1062) (sqlstate HY000)
QUERY:          INSERT INTO clstr_1 VALUES (1);
RECEIVED ERROR: duplicate primary key given: [1] (errno 1062) (sqlstate HY000)
QUERY:          INSERT INTO clstr_2 VALUES (2);
RECEIVED ERROR: duplicate primary key given: [2] (errno 1062) (sqlstate HY000)
QUERY:          INSERT INTO clstr_2 VALUES (1);
RECEIVED ERROR: duplicate primary key given: [1] (errno 1062) (sqlstate HY000)
QUERY:          INSERT INTO clstr_3 VALUES (2);
RECEIVED ERROR: duplicate primary key given: [2] (errno 1062) (sqlstate HY000)
QUERY:          INSERT INTO clstr_3 VALUES (1);
RECEIVED ERROR: duplicate primary key given: [1] (errno 1062) (sqlstate HY000)
QUERY:          DELETE FROM clstr_1;
RECEIVED ERROR: table not found: check_seq (errno 1146) (sqlstate HY000)
QUERY:          INSERT INTO clstr_1 VALUES (2);
RECEIVED ERROR: duplicate primary key given: [2] (errno 1062) (sqlstate HY000)
QUERY:          INSERT INTO clstr_1 VALUES (1);
RECEIVED ERROR: duplicate primary key given: [1] (errno 1062) (sqlstate HY000)

collate.icu.utf8

QUERY:          CREATE DOMAIN testdomain AS text;
RECEIVED ERROR: type "testdomain" already exists (errno 1105) (sqlstate HY000)

copy

QUERY:          truncate parted_copytest;
RECEIVED ERROR: table not found: myint (errno 1146) (sqlstate HY000)
QUERY:          truncate parted_copytest;
RECEIVED ERROR: table not found: myint (errno 1146) (sqlstate HY000)
QUERY:          truncate parted_copytest;
RECEIVED ERROR: table not found: myint (errno 1146) (sqlstate HY000)
QUERY:          truncate table parted_copytest;
RECEIVED ERROR: table not found: myint (errno 1146) (sqlstate HY000)

copy2

QUERY:          TRUNCATE vistest;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          TRUNCATE vistest;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          COPY vistest FROM stdin CSV;
RECEIVED ERROR: expected a DoltTransaction
QUERY:          TRUNCATE vistest;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          TRUNCATE vistest;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          TRUNCATE vistest;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          TRUNCATE vistest;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          TRUNCATE vistest;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)
QUERY:          TRUNCATE vistest;
RECEIVED ERROR: table not found: ss1 (errno 1146) (sqlstate HY000)

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

copy2

QUERY: ROLLBACK TO SAVEPOINT s1;

enum

QUERY: DELETE FROM enumtest_parent;

foreign_key

QUERY: delete from pktable2;

plpgsql

QUERY: delete from HSlot;

truncate

QUERY: TRUNCATE TABLE truncate_a;
QUERY: TRUNCATE TABLE trunc_c;
QUERY: TRUNCATE TABLE trunc_c;
QUERY: TRUNCATE ONLY truncparted;
QUERY: TRUNCATE ONLY truncparted;
QUERY: TRUNCATE TABLE truncprim;

updatable_views

QUERY: DELETE FROM base_tbl;
QUERY: DELETE FROM base_tbl;

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
Copy Markdown
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, happy with the overall look. Lots of small comments.

The biggest comment is something you don't need to address as part of this PR: we really need to not be serializing the compiled form of functions.

@Hydrocharged Hydrocharged enabled auto-merge March 28, 2025 12:47
@Hydrocharged Hydrocharged merged commit 2b8d6ad into main Mar 28, 2025
14 checks passed
@Hydrocharged Hydrocharged deleted the daylon/root-merging branch March 28, 2025 13:05
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