Skip to content

Rewrote casts as a root object collection#2566

Open
Hydrocharged wants to merge 1 commit intomainfrom
daylon/cast-rewrite
Open

Rewrote casts as a root object collection#2566
Hydrocharged wants to merge 1 commit intomainfrom
daylon/cast-rewrite

Conversation

@Hydrocharged
Copy link
Copy Markdown
Collaborator

This removes casts from being strictly built-in functionality to now being a root object collection, which will allow for users to be able to use CREATE CAST.

With this PR, there are two major pieces of work that still need to be done.

  1. We need to thread a *sql.Context everywhere in GMS. sql.Node functions such as WithChildren do not currently take a context, and we've tried to hack around this for too long by storing contexts inside of the struct itself, but that can only go so far. For now, I have a hack that creates a faux collection in the presence of a nil context, and that collection only allows access to the built-in casts.
  2. We currently don't persist changes to the cast root object collection. We have an issue where, upon root creation, all root object collections are persisted as simply being nil in the root. However, once the collection has been loaded (or created on first load), they end up trying to persist which actually changes the root, as it will overwrite the nil with an empty value instead, which changes the hash. This (incorrectly) can trigger our dual-branch-modification check, which effectively prevents future updates in a session. Attempting to write empty values upon root creation isn't working for some reason, so I'm still investigating the best way to proceed. This is a pre-existing bug that hasn't been caught as all tests that feature multiple branches have not used the other root object collections.

@Hydrocharged Hydrocharged requested a review from fulghum April 10, 2026 10:34
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 10, 2026

Main PR
covering_index_scan_postgres 1202.30/s 1165.93/s -3.1%
index_join_postgres 203.35/s ${\color{red}111.96/s}$ ${\color{red}-45.0\%}$
index_join_scan_postgres 283.74/s 281.17/s -1.0%
index_scan_postgres 14.83/s 14.65/s -1.3%
oltp_point_select 2906.10/s ${\color{red}2597.99/s}$ ${\color{red}-10.7\%}$
oltp_read_only 2276.42/s 2078.63/s -8.7%
select_random_points 175.87/s 172.86/s -1.8%
select_random_ranges 884.58/s ${\color{red}471.65/s}$ ${\color{red}-46.7\%}$
table_scan_postgres 14.35/s 14.17/s -1.3%
types_table_scan_postgres 6.52/s 6.42/s -1.6%

@github-actions
Copy link
Copy Markdown
Contributor

Main PR
Total 42090 42090
Successful 17942 17942
Failures 24148 24148
Partial Successes1 5636 5636
Main PR
Successful 42.6277% 42.6277%
Failures 57.3723% 57.3723%

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.

@Hydrocharged
Copy link
Copy Markdown
Collaborator Author

Performance metrics are likely impacted by the lack of context threading, which is triggering a full collection build every time. That will be fixed once I’ve finished threading the context everywhere in GMS.

Copy link
Copy Markdown
Contributor

@fulghum fulghum left a comment

Choose a reason for hiding this comment

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

LGTM

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