Skip to content

use :prefix to query attached databases - #183

Merged
warmwaffles merged 1 commit into
elixir-sqlite:mainfrom
mcha-forks:mochaa/prefix
Sep 30, 2026
Merged

warmwaffles merged 1 commit into
elixir-sqlite:mainfrom
mcha-forks:mochaa/prefix

Conversation

@mochaaP

@mochaaP mochaaP commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Reverts "raise on table prefixes" (71ddd0a, 7882a01, #103).

Supersedes #175, thanks @sbaildon.

@mochaaP

mochaaP commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor Author

to maintainers: you may also want to pick changes from mochaa/main.

@mochaaP
mochaaP force-pushed the mochaa/prefix branch 7 times, most recently from cfce3b8 to ad3b608 Compare September 30, 2026 09:42
@mochaaP

mochaaP commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author
f41a2de (HEAD -> main, mochaa/main) Support modifiers and drop mode
14b616f Stricter testing for raised error messages
4942046 Support table-level check constraints
61cdfa3 Fix incorrectly named BinaryUUIDTest that didn't get loaded
aa13a6f Remove deprecated parentheses after field access usage in tests
ad3b608 (mochaa/mochaa/prefix, mochaa/prefix) use :prefix to query attached databases

Comment thread CHANGELOG.md Outdated

- changed: Reimplement querying prefix names.
- added: Raise on cross-database foreign keys.
- changed: **breaking** Made quote_name/1 private.

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.

Not really a breaking change. If someone was relying on that obscure function on the adapter from their application. That's a them problem. 🤣

Suggested change
- changed: **breaking** Made quote_name/1 private.
- changed: Made `quote_name/1` private.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

agree. 🤪

@warmwaffles

Copy link
Copy Markdown
Member

Is there any way on the integration side you can add a prefix test or two specifically for sqlite. I just want to make sure this functionality works and stays working going forward. You are free to construct schemas and stuff however you want for it.

@mochaaP

mochaaP commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Is there any way on the integration side you can add a prefix test or two specifically for sqlite. I just want to make sure this functionality works and stays working going forward. You are free to construct schemas and stuff however you want for it.

in integration_test/? sure, will do later.

@warmwaffles

Copy link
Copy Markdown
Member

Yes, anywhere in there is fine with me. Construct it however you want.

@mochaaP

mochaaP commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

not sure how should I do that, ATTACH DATABASE is per connection and current test repo pool size is 5

@warmwaffles

Copy link
Copy Markdown
Member

Actually that migration test should be sufficient enough since that's executed in the integration tests.

@mochaaP

mochaaP commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

also tests are a bit flaky because sqlite expects single writer.

@mochaaP

mochaaP commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

well, does this count?

defmodule Ecto.Integration.PrefixTest do
  use Ecto.Integration.Case, async: true

  import Ecto.Query, only: [from: 2]

  alias Ecto.Integration.Post
  alias Ecto.Integration.TestRepo

  test "queries an attached database through :prefix" do
    TestRepo.query!(~s|ATTACH DATABASE ":memory:" AS "foo"|)
    TestRepo.query!(~s|CREATE TABLE "foo"."posts" AS SELECT * FROM "main"."posts"|)

    TestRepo.insert!(%Post{id: 1}, prefix: "foo")

    results =
      from(Post, prefix: "foo")
      |> TestRepo.all()

    assert [%Post{id: 1}] = results
  end
end

Revert "raise on table prefixes"

This reverts commit 71ddd0a.

As a side effect, the quoting changes also
Fixes: eca58dd ("Add support for :check constraint at column level (elixir-sqlite#29)")
@warmwaffles

Copy link
Copy Markdown
Member

Yea that works

@warmwaffles
warmwaffles merged commit d660df7 into elixir-sqlite:main Sep 30, 2026
8 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