Add indices support (WIP) #335

Martoon-00 · 2018-12-10T17:10:02Z

Motivation

Currently, there is no way to deal with the database schema purely via Haskell code, which is not good at least because when we define our schema in raw SQL, we get table and fields names defined in several places.
I'm planning to make SQL engine know about foreign key constraints in the next PR, and these two seem to provide a basis for moving away from manual schemas definition completely.

PR Details

The following features are implemented:

Manual indices definition.
Here a user can provide a list of indices on the per-table basis, each index is defined by a sequence of its fields and its settings (only most basic settings are supported for now). With syntax as the following.
Automatic indices detection.
A user can make a DatabaseIndices (which is similar to DatabaseSettings but contains information about indices correspondingly) via defaultDbIndices, which gathers indices on those columns which are assumed to participate in JOINs.
Later the user can add these indices to the custom ones via dedicated function mergeDbIndices, or immediately use addDefaultDbIndices.
Manual/automatic migrations support.
Previously gathered indices are converted to TableChecks and added to CheckedDatabaseSettings.
Two action providers are supplied: ADD INDEX and DROP INDEX.
Herewith, unfortunately, there is no SQLite support, since there is no way to modify indices of the existing table there. A common workaround is to rename the table, create a new one with the desired indices and copy all the data, but I'm not yet ready to write copying action providers, although generally, that scheme seems to be perfectly discoverable with existing automatic migration means.
Testing.
- Unit tests on indices definition.
- Manual migrations.
- Automatic migrations.
Haddock: builds.
Extend the manual.

Notes

I have a set of TODOs remained where I'm not sure about proper design choice, would appreciate hearing your point of view on this.

Also, I added .stylish-Haskell.yaml which corresponds to the style of this project, otherwise I get pretty mad diffs each time I save a file.

tathougies · 2018-12-11T05:32:15Z

Hello!

Wow.. Thanks! This is a lot to digest, but right off the bat, I'll ask you to rebase on top of the beam-0.8.0.0 branch, since that represents the next release. There'll be a few changes you'll have to make to get it compiling. The upside is that it should trigger a gitlab CI build, which will have to pass before I merge.

Additions to the manual would be nice.

I'm still getting through the code.

Travis

tathougies · 2018-12-11T05:33:15Z

beam-core/Database/Beam/Schema/Indices.hs

+
+import           Database.Beam.Schema.Tables
+
+-- Some day it should have more options and allow to modify them depending on the backend.


When merged, add a ticket for this to the issue reporting system

tathougies · 2018-12-11T05:35:54Z

beam-core/Database/Beam/Schema/Indices.hs

+-- instances for all of them.
+-- | Traverses the given table and for every field which is some 'PrimaryKey'
+-- makes corresponding SQL index, this allows "JOIN"s on this table perform nicely.
+instance {-# OVERLAPPABLE #-}


Why are these instances overlappable?

Well, I'm not sure whether is it a good choice, but I saw that there are plans to support many different entities type, while we want to have indices only for TableEntities. It looked like the one who will enable support for those other entities should not bother about indices at all, thus a default overlappable instance.
Should I define instances for each entity type explicitly instead?

tathougies · 2018-12-11T05:42:43Z

beam-core/Database/Beam/Schema/Indices.hs

+-- | Provide options for an automatically created index, which is caused by a primary key
+-- of one table being embedded into another.
+-- This typeclass is only needed to be defined for inter-table references (see ':->').
+class IndexFromReference reference where


The issue I see right now is that tables should be able to have two foreign keys that refer to the same table type at the haskell level, but really refer to two different tables at the database level.

For example

data TwoKeyT f = TwoKey { _key1 :: PrimaryKey RefT f _key2 :: PrimaryKey RefT f } data RefT f = Ref { _field1 :: C f Text, _field2 :: C f Int } data Database entity = Database { _ref1 :: entity (TableEntity RefT), _ref2 :: entity (TableEntity RefT), _twoKey :: entity (TableEntity TwoKeyT) }

How would I make it so that it knows that _key1 refers to _ref2 and _key2 to _ref1 (or vice versa). I believe the way it is now, each type is uniquely linked to one database table, which is a large constraint.

One way to get around this that would also (I think) have the benefit of removing the instance overlap is to not use the type class mechanism and require the user to construct an appropriate value.

I agree this is a major problem. In case of this implementation via type class mechanism a workaround is to parametrize table type with a phantom type parameter, thus making different tables of the same type distinguishable in a Haskell code.
However, making user explicitly make a construct which describes options for the generated indices seems to be virtually better in several ways. Doing so may be tricky if we still want the type system to enforce the user to specify indices for all embedded PrimaryKeys present; I described my raw suggestion right below this code, we could discuss it :)

Martoon-00 · 2018-12-11T08:00:38Z

Oh, having CI checking my work would be really nice!
I see beam-0.8.0.0-fix-ci branch and multiple travis/beam-0.8.0.0-* braches, which one should I choose?

tathougies · 2018-12-11T08:05:40Z

Just regular beam-0.8.0.0. I'll get rid of the other ones. They're all merged.

Martoon-00 · 2018-12-11T09:19:36Z

That's weird, I see no plain beam-0.8.0.0 branch 🤔

tathougies · 2018-12-11T17:12:50Z

This one: https://github.com/tathougies/beam/tree/travis/beam-0.8.0.0

Martoon-00 · 2018-12-11T17:25:16Z

Ah, thanks. I wasn't sure it was that branch you were talking about because beam-core tests fail to compile there (713522b commit). Could you check?

tathougies · 2018-12-13T18:47:55Z

The latest commit there should be 377596e

Martoon-00 · 2018-12-13T19:36:08Z

Oh right, I searched in my forked repository. Thanks for helping with figuring this out, I'm rebasing.

tathougies · 2018-12-18T17:16:53Z

You'll need to resolve conflicts too to get CI working.

Martoon-00 · 2018-12-18T21:54:59Z

Yeah, it's gonna take a while :)

This includes: * Manual indices definition; * Automatic detection of indices required for performant JOINs; * Manual and automatic index creation/deletion via `ADD/DROP INDEX` (without SQLite support)

Martoon-00 · 2018-12-20T11:35:17Z

Finally, I rebased upon beam-0.8.0.0 branch. Now there are a couple of small issues:

CI here is not triggered, probably because I'm proposing this change from another repo.
Is there an option to enable CI in this case and could you do that, or I'd better relocate my code to this repository?
I activated CI in my fork for now, and sometimes tests fail https://travis-ci.org/serokell/beam/jobs/470453508.
Postgres seems to happily truncate strings after \0 character.

tp-woven · 2022-08-08T05:03:10Z

Oh, wow, this looks really cool. Any chance of this getting resurrected and merged?

kmicklas · 2022-08-13T07:38:09Z

@tp-woven You're welcome to take it over! I'm happy to review but unfortunately don't have the time for major feature development on Beam these days.

Martoon-00 · 2022-08-31T03:02:44Z

Yeah, @tp-woven I would be glad if you manage to take this work or pick it as inspiration for your own implementation, especially since @kmicklas approved this. Now I cannot allocate time to this.

If I remember correctly, this PR mostly handles the core part (not tested at all) but may need a decent update of the migration package.

tp-woven · 2022-08-31T03:19:58Z

Thanks, I'll try to take over. Fair warning: I'm a Haskell noob, so it may take a while... 😅

Martoon-00 force-pushed the martoon/add-indices branch 2 times, most recently from 3e9e642 to 287341f Compare December 10, 2018 20:36

tathougies requested changes Dec 11, 2018

View reviewed changes

Add .stylish-haskell.yaml

f9c76f0

Martoon-00 force-pushed the martoon/add-indices branch from d4a3c4d to 0bb3279 Compare December 13, 2018 19:19

Martoon-00 added 2 commits December 20, 2018 13:14

Add indices support

0ac91ca

This includes: * Manual indices definition; * Automatic detection of indices required for performant JOINs; * Manual and automatic index creation/deletion via `ADD/DROP INDEX` (without SQLite support)

Implement predicateRestrictsDropOf

f861d40

Martoon-00 force-pushed the martoon/add-indices branch from 0bb3279 to f861d40 Compare December 20, 2018 10:14

Martoon-00 changed the base branch from master to travis/beam-0.8.0.0 December 20, 2018 10:15

Martoon-00 force-pushed the martoon/add-indices branch from fe622cd to f861d40 Compare December 20, 2018 10:25

Fix CI

6798056

Martoon-00 force-pushed the martoon/add-indices branch from d0f96d1 to 6798056 Compare December 20, 2018 11:11

kmicklas added the enhancement label Jun 10, 2020

kmicklas marked this pull request as draft June 8, 2024 09:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add indices support (WIP) #335

Add indices support (WIP) #335

Martoon-00 commented Dec 10, 2018 •

edited

Loading

tathougies commented Dec 11, 2018

tathougies Dec 11, 2018

tathougies Dec 11, 2018

Martoon-00 Dec 11, 2018

tathougies Dec 11, 2018

Martoon-00 Dec 11, 2018

Martoon-00 commented Dec 11, 2018

tathougies commented Dec 11, 2018

Martoon-00 commented Dec 11, 2018

tathougies commented Dec 11, 2018

Martoon-00 commented Dec 11, 2018

tathougies commented Dec 13, 2018

Martoon-00 commented Dec 13, 2018

tathougies commented Dec 18, 2018

Martoon-00 commented Dec 18, 2018

Martoon-00 commented Dec 20, 2018 •

edited

Loading

tp-woven commented Aug 8, 2022

kmicklas commented Aug 13, 2022

Martoon-00 commented Aug 31, 2022

tp-woven commented Aug 31, 2022


		import Database.Beam.Schema.Tables

		-- Some day it should have more options and allow to modify them depending on the backend.

Add indices support (WIP) #335

Are you sure you want to change the base?

Add indices support (WIP) #335

Conversation

Martoon-00 commented Dec 10, 2018 • edited Loading

Motivation

PR Details

Notes

tathougies commented Dec 11, 2018

tathougies Dec 11, 2018

Choose a reason for hiding this comment

tathougies Dec 11, 2018

Choose a reason for hiding this comment

Martoon-00 Dec 11, 2018

Choose a reason for hiding this comment

tathougies Dec 11, 2018

Choose a reason for hiding this comment

Martoon-00 Dec 11, 2018

Choose a reason for hiding this comment

Martoon-00 commented Dec 11, 2018

tathougies commented Dec 11, 2018

Martoon-00 commented Dec 11, 2018

tathougies commented Dec 11, 2018

Martoon-00 commented Dec 11, 2018

tathougies commented Dec 13, 2018

Martoon-00 commented Dec 13, 2018

tathougies commented Dec 18, 2018

Martoon-00 commented Dec 18, 2018

Martoon-00 commented Dec 20, 2018 • edited Loading

tp-woven commented Aug 8, 2022

kmicklas commented Aug 13, 2022

Martoon-00 commented Aug 31, 2022

tp-woven commented Aug 31, 2022

Martoon-00 commented Dec 10, 2018 •

edited

Loading

Martoon-00 commented Dec 20, 2018 •

edited

Loading