oldmoe / litestack

MIT License
1.02k stars 56 forks source link

Use rowid instead id, allowing custom primary keys #117

Closed gabriel-curtino closed 2 months ago

gabriel-curtino commented 3 months ago

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

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:

oldmoe commented 2 months ago

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 commented 2 months ago

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