Skip to content

Remove duplicated prepare#352

Merged
warmwaffles merged 2 commits into
elixir-sqlite:mainfrom
katafrakt:remove-duplicated-prepare
Jul 14, 2026
Merged

Remove duplicated prepare#352
warmwaffles merged 2 commits into
elixir-sqlite:mainfrom
katafrakt:remove-duplicated-prepare

Conversation

@katafrakt

Copy link
Copy Markdown
Contributor

I started reading connection.ex to understand query caching story (before reading in the readme that it's not supported; hours of reading the code can save minutes of reading the docs). During that I noticed that there is prepare and prepare_no_cache that are basically identical and definitely both are not caching. I figures this is some leftover from adding the cache in the past. From #41 I understand it's not coming back, so perhaps it's ok to clean this up.

Also fixes some typespecs that are not in line with the implementation.

It looks like a remnant from a time when adding query caching was
attempted, but later was abandoned.
@warmwaffles

Copy link
Copy Markdown
Member

I wouldn't say it coming back is impossible. I just tossed it at the time because I envisioned access patterns where the queries would vary in composition.

@warmwaffles warmwaffles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks reasonable.

Curious, how did you find the incorrect typespecs?

@warmwaffles warmwaffles merged commit cbda961 into elixir-sqlite:main Jul 14, 2026
9 checks passed
@katafrakt

Copy link
Copy Markdown
Contributor Author

I'm afraid there's no secret sauce to it, I just tend to sometimes spot such things. I don't claim everything is correct now.

@katafrakt katafrakt deleted the remove-duplicated-prepare branch July 14, 2026 14:47
@warmwaffles

Copy link
Copy Markdown
Member

Well I appreciate it. I usually can spot those as well, but sometimes callbacks like this slip through because the typespec is terse.

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