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

Use rowid instead id, allowing custom primary keys #117

Merged
merged 6 commits into from
Jun 22, 2024

Conversation

gabriel-curtino
Copy link
Contributor

@gabriel-curtino gabriel-curtino commented Jun 4, 2024

This PR intent is improve liteseach making it rely in the rowid instead the id as it does actually. This will allow to work with custom TEXT primary keys and maybe clear the way for composite keys.

I've found that

  • "Whenever you create a table without specifying the WITHOUT ROWID option, you get an implicit auto-increment column called rowid. The rowid column store 64-bit signed integer that uniquely identifies a row in the table."
  • "If a table has the primary key that consists of one column, and that column is defined as INTEGER then this primary key column becomes an alias for the rowid column."

Knowing this, if we use the rowid instead the id column, it doens't matter what kind of primary key we use.

It also adds the possibility of set a custom primary key name, then this kind of setup will work:

class CreateUsers < ActiveRecord::Migration[7.1]
  def change
    create_table :users, id: false do |t|
      t.string :user_id, null: false, primary_key: true, default: -> { "lower(hex(randomblob(16)))" }
      t.string :name

      t.timestamps
    end
  end
end

class CreatePosts < ActiveRecord::Migration[7.1]
  def change
    create_table :posts, id: false do |t|
      t.string :post_id, null: false, primary_key: true, default: -> { "lower(hex(randomblob(16)))" }
      t.references :author, type: :string
      t.string :title
      t.text :content

      t.timestamps
    end
  end
end

class User < ApplicationRecord
  self.primary_key = "user_id"

  has_many :posts, foreign_key: :author_id

  include Litesearch::Model
  litesearch do |schema|
      schema.fields %w[ name ]
      schema.primary_key :user_id
  end
end

class Post < ApplicationRecord
  self.primary_key = "post_id"

  belongs_to :author, class_name: "User", primary_key: :user_id

  include Litesearch::Model
  litesearch do |schema|
      schema.fields %w[ title content ]
      schema.field :author, target: "users.name", primary_key: :user_id
      schema.primary_key :post_id
  end
end

Notes:

  • Test pass, but I didn't add this kind of scenario for sequel tests. Need some help there.
  • To get similar to work with the rowid, it does an extra query to read the rowid of the object. There is the alternative to "expose" the rowid attribute adding a default scope in the models when including litesearch, but I think too many things can go wrong doing it and one extra query just for running similar doesn't looks too bad.

@oldmoe
Copy link
Owner

oldmoe commented Jun 9, 2024

I have looked into rowid stability as per my comment in #112 and it seems rowids are still not guaranteed to remain stable after a vacuum. see the quirks section of the rowid docs. Relying on rowid would mean users need to rebuild the index after any vacuum operation or they might get the wrong results.

@oldmoe
Copy link
Owner

oldmoe commented Jun 13, 2024

I still think it is the right thing to do though, this will expand the support to any rowid table, and we could highlight the potential need to rebuild the index after the vacuum in the documentation

@oldmoe oldmoe merged commit c579a87 into oldmoe:master Jun 22, 2024
0 of 6 checks passed
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