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

Incomplete PRAGMA support #559

Open
rickhull opened this issue Sep 16, 2024 · 9 comments
Open

Incomplete PRAGMA support #559

rickhull opened this issue Sep 16, 2024 · 9 comments

Comments

@rickhull
Copy link
Contributor

rickhull commented Sep 16, 2024

I have noticed some inconsistencies in the handling for SQLite pragmas.

  1. There is a setter for busy_timeout= but no getter. Likewise for ignore_check_constraints=.
  2. No accessors for analysis_limit hard_heap_limit trusted_schema
  3. No method for optimize
  4. No methods for function_list module_list pragma_list
  5. No methods for table_list table_xinfo

For the last, SQLite supports 3 pragmas for index and table:

  • thing_list
  • thing_info
  • thing_xinfo

The gem I have installed, version 2.0.4, has methods for all three index pragmas but only the table_info method.

Some of these may not make sense to support, but these are my findings. Separately, I have some ideas about how to reorganize lib/sqlite3/pragmas.rb.

@flavorjones
Copy link
Member

@rickhull Thank you for pointing this out!

If you'd like to get some easy PRs in, I'd be very grateful for your help with these! If not, I'll leave this open as a note for other contributors.

@rickhull
Copy link
Contributor Author

I might take a stab. Here's what I'm doing in the meantime: https://gist.github.com/rickhull/464c6fdf5fdea217894f67618b4a2b0c

@rickhull
Copy link
Contributor Author

rickhull commented Oct 3, 2024

Please take a look at the Nostrb::SQLite::Pragma class: https://github.com/rickhull/nostrb/blob/master/lib/nostrb/sqlite.rb#L6

I want to basically import this wholesale, or something like it, as our pragma backend. We can then patch or rewrite the current API using this backend. I think it will be relatively easy to delete nearly of the current pragma impl with this more programmatic approach.

Even if not, I may still look at extending the current approach.

@flavorjones
Copy link
Member

@rickhull I'm not opposed to doing something like what you're describing, but maybe we can get there iteratively instead of big-bang copying code and then trying to adapt to it.

For example, I really like how that class defines the pragma setters and getters dynamically. Maybe we could start by introducing that technique into this gem (and adding some tests for those pragmas)? I think that really cleans things up and addresses almost all of the initial issues you raised above. WDYT?

@rickhull
Copy link
Contributor Author

rickhull commented Oct 6, 2024

Totally agreed on not necessarily big-bang-adapt. If I can land a suitable patch here, then I can delete most of my stuff. Let me come up with something more concrete in the next few days.

@rickhull
Copy link
Contributor Author

rickhull commented Oct 6, 2024

How do we feel about these pragmas I've ignored?

      #      debug: parser_trace schema_version stats vdbe_* writable_schema
      #     legacy: legacy_alter_table legacy_file_format
      # deprecated: case_sensitive_like count_changes data_store_directory
      #             default_cache_size empty_result_callbacks full_column_names
      #             short_column_names temp_store_directory

My take is that they are all easily available without explicit support via: Database#execute("PRAGMA ...")

@flavorjones
Copy link
Member

@rickhull I don't mind if some pragmas end up unimplemented, but I do want to make sure we don't remove existing pragmas. I didn't do a complete check, but for example default_cache_size is currently supported by this gem. Does that make sense?

@rickhull
Copy link
Contributor Author

Sounds good! I'm pretty slammed for the next couple weeks but I'm 90% of the way to a PR. It may take a lil while to get there.

@rickhull
Copy link
Contributor Author

rickhull commented Dec 31, 2024

I haven't forgotten about this and am willing to support and guide if someone else wants to drive. Otherwise I'm 90% of the way to a plan for a PR, but need to carve out the time to crank it out. Probably not for 2024 though. It's still on the back burner though.

The 90% plan is, at essence:

  • adopt the strategy from Nostrb
  • adapt the strategy to the sqlite3-ruby environment
  • include all currently supported pragmas
  • get tests to pass

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

No branches or pull requests

2 participants